feat(astro): Exposes extra APIs for script and testing #12052
+47
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 7cac830 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
matthewp
approved these changes
Sep 22, 2024
florian-lefebvre
approved these changes
Sep 28, 2024
bluwy
requested changes
Sep 29, 2024
Comment on lines
-43
to
+45
| * @internal only used for testing | ||
| * When building multiple projects in the same execution (e.g. during tests), disabling | ||
| * this option can greatly improve performance at the cost of some extra memory usage. | ||
| * |
There was a problem hiding this comment.
I'd prefer to keep this internal still. I imagine for external libraries this shouldn't have a significant impact if not used since it only mattered for Astro having tons of tests.
There was a problem hiding this comment.
Astro has around 2,000 tests. For companies requiring high reliability, it is not uncommon to have very exhaustive testing on libraries (at least for the companies I know that fit that description, including mine).
Multiple of our libraries for Next have over 10k tests each. Libraries for Astro (which we are working on) won't be any less tested.
There was a problem hiding this comment.
This isn't related to the number of test, but the number of builds for tests. Astro has many builds to test different permutations of all features and I doubt the same is needed for external libraries. Even so, Astro's test suite is due for some fixtures reduction because some fixtures could be combined, so in practice this option isn't really encouraged.
Comment on lines
+62
to
+69
| /** | ||
| * Turn raw config values into normalized values | ||
| */ | ||
| export function validateConfig( | ||
| userConfig: any, | ||
| root: string, | ||
| cmd: string, | ||
| ): Promise<AstroConfig>; |
There was a problem hiding this comment.
What's the usecase for exporting this? I'm not too comfortable with exposing this yet since the validation doesn't take in account of --verbose and --silent (or log level in the future) and it always logs to the console when it shouldn't in tests, so it requires some more refactor.
There was a problem hiding this comment.
It's one of the APIs necessary to implement testing properly. Otherwise, we'd need to re-implement the transformations fone in the schema.
It is mentioned at the end of the proposal:
Similarly, the function
validateConfigthat fills default values and apply transformations on the configuration is also not exported. This is required to resolve a value from the configuration that is not directly set on the test but used for the test tolling, like theoutDirandsrcDir.
it always logs to the console when it shouldn't in tests, so it requires some more refactor.
I don't see anywhere in the code where it would log anything. It creates a Zod schema and parses using it. None of the refine, superRefine and transform functions in the schema has any logs, they just add possible issues which will be reported as an error to the caller not as logs.
Are you perhaps thinking of resolveConfig?
There was a problem hiding this comment.
Ah yeah it's resolveConfig that's logging the error, not validateConfig. But in that case, wouldn't resolveConfig be more useful as it mimics what dev() and build() loads for the astro config? validateConfig() only handles the validation part.
|
!preview test-apis |
|
Snapshots have been released for the following packages:
Publish LogBuild Log |
ascorbic
reviewed
Oct 8, 2024
ematipico
approved these changes
Oct 14, 2024
Co-authored-by: Matt Kane <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Testing
There was no change in logic. The behavior for all changed functions was already tested.
Docs
buildAPI should be updatedastro/config, so nothing to update. The two functions newly exported would go in such a reference.