Refactored wp-env Config Parsing
#50071
Conversation
This commit changes the way that configuration parsing works in order to better facilitate the creation of root-only configuration options. This does, however, leave `wp-env` in a non-functional state. Now that the config parsing itself is done, I wanted to commit it. I almost accidentally deleted this once today and that was too many times!
This commit adds basically all of the other tests from `tests/config.js` in their respective test files. This covers the last of the tests and the config parsing should now have the same or better level of test coverage.
In order to create the `WPConfig` object we will switch to composing the output from config parsing into a new class. This lets us avoid having to rewrite all of the tests to get a different output and continue with the trend of composing together the parsing logic.
This commit finishes the refactor and hooks up the loading of config files using the new functionality.
Since we're already handling this in the new parser, there's no need to perform this translation here.
|
Also, you make a lot of good points above :) My subjective take is that some of this makes the code more verbose than it really needs to be. Which isn't a huge problem, but I think there are one or two layers of indirection that are a bit difficult to grok. That said, I think when we get some documentation in place (like "how to add a config key" or "how to add an env var override"), it really won't be a problem |
These types will be more descriptive of their purpose.
|
Thanks for the review @noahtallen! I'm actually pretty happy about the above test failure because it shows that the test suite is really robust. When I removed the I also aded an integration test for the environment variable overrides so that the tests cover all four configuration states. |
|
The Puppeteer 3 suite seems to be failing across all commits right now, unfortunately. |
| expect( parsed ).toEqual( { | ||
| ...DEFAULT_CONFIG, | ||
| coreSource: { | ||
| basename: 'gutenberg', |
There was a problem hiding this comment.
@ObliviousHarmony @noahtallen One side-effect of hard-coding the basename (and also testPath) like this is is that if someone checks out the gutenberg repo into a differently named folder, these tests will fail.
This is currently causing a few issues for CI jobs that run on security patches, as those are created on a differently named private repo.
I was wondering what your thoughts on a good fix would be?
A naive option could be expect.any( String ), but I think maybe this should all be mocked rather than being based off of the real file system.
There was a problem hiding this comment.
That's an interesting case @talldan. It seems like the best source would be to just generally avoid any relative paths and use fake absolute paths in our tests. I've gone ahead and created an issue for this.
What?
This pull request refactors the existing
.wp-env.jsonconfig file parsing in order to more cleanly support root properties. It aimed to increase composition and testability while maintaining full compatibility.Why?
While working on #49996 we decided that it would be best if the
postInstallscript existed at the configuration root. When I looked into implementing that, however, it proved to be troublesome due to the way config parsing was designed. It was explicitly created to cascade the config file almost immediately, and as a result, it was not easy to introduce root-level options and validation in a way that would not cascade.How?
We delay the cascade of options from the root to the environments until after validation and parsing have completed. This lets us interact with the config object while the root and environments are both present but separate. As a result we will be able to support root-level configuration options without any additional changes.
I was worried about breaking compatibility, so, this pull request also features an exhaustive test suite update. All new functionality is more or less completely covered for both happy path and failure states. This was a great exercise and caught a number of bugs in the process.
Testing Instructions
npm run env -- start --updateand make sure that both the development and tests environments work.npm run test:unit packages/envand make sure that they pass..wp-env.override.jsonfile and make sure that it overrides as expected. Config and mappings should be merged while plugins and themes are overridden.