Open david-szabo97 opened 3 years ago
I'll add that even more disruptively, Gutenberg's end-to-end tests were broken for a while after TT1 Blocks removed their front-page
template (https://github.com/WordPress/theme-experiments/pull/179) -- which our e2e tests relied on. This was thankfully fixed on the Gutenberg side by @Addison-Stavlo in https://github.com/WordPress/gutenberg/pull/28638.
However, it's evidence that we shouldn't rely on external dependencies for our day-to-day quality assurance -- in the form of e2e tests, which we want people to trust, rather than ignore.
@jeyip and I just discussed this in Slack. We came to the conclusion that the key issue here is reproducibility: Depending on a moving target -- such as the https://github.com/WordPress/theme-experiments repo -- makes unpredictable with what actual theme code we end up with (since that implicitly refers to the HEAD
of the master
branch of that repo, which is constantly changing).
https://github.com/WordPress/gutenberg/blob/0105c9a2312272d768f1ecb77c0b9e6dca77d0a0/.wp-env.json#L6
A viable fix for this might be to pin this to a certain version instead. Fortunately, it seems like wp-env
supports using git refs (such as commit SHAs and branch or tag names).
So we might actually get away with pinning to e.g. https://github.com/WordPress/theme-experiments/commit/f3d2c0dac4dd6a127b177178c5145c10f28401af (and maybe we can later convince theme-experiments
folks to tag theme versions, e.g. tt1-blocks@0.4.3
).
One final caveat: I'm not 100% sure if wp-env
supports both pinning a GH repo to a given ref, and specifying a subdirectory of that repo (as we need to do here for tt1-blocks
).
Well wp-env
's config parsing seems to work anyway :thinking: (Tests pass with the following change.)
diff --git a/packages/env/lib/config/test/config.js b/packages/env/lib/config/test/config.js
index c9d620b12d..a20bf54569 100644
--- a/packages/env/lib/config/test/config.js
+++ b/packages/env/lib/config/test/config.js
@@ -366,6 +366,7 @@ describe( 'readConfig', () => {
'WordPress/gutenberg',
'WordPress/gutenberg#master',
'WordPress/gutenberg#5.0',
+ 'WordPress/theme-experiments/tt1-blocks#f3d2c0d',
],
} )
)
@@ -394,6 +395,16 @@ describe( 'readConfig', () => {
path: expect.stringMatching( /^\/.*gutenberg$/ ),
basename: 'gutenberg',
},
+ {
+ type: 'git',
+ url:
+ 'https://github.com/WordPress/theme-experiments.git',
+ ref: 'f3d2c0d',
+ path: expect.stringMatching(
+ /^\/.*theme-experiments\/tt1-blocks$/
+ ),
+ basename: 'tt1-blocks',
+ },
],
};
expect( config.env.tests ).toMatchObject( matchObj );
This is a problem in communication about the purpose and scope of TT1 Blocks when it was created, as it was never even stated to the developers (hi) that this theme was meant to be used this way.
You can guess this lengthens the time it takes to deliver a PR since we need to wait for the changes in TT1B. We need everyone to clearly express what they need, not sit and wait.
Hey @carolinan! :wave: Thanks for stopping by!
It's true that different stakeholders have different needs and use cases, and we don't always state those clearly when we start using each other's projects.
That said, I'll give a try to the pinned dependency approach I was outlining in my other comment, as I think it might be a viable solution.
So we might actually get away with pinning to e.g. https://github.com/WordPress/theme-experiments/commit/f3d2c0dac4dd6a127b177178c5145c10f28401af (and maybe we can later convince
theme-experiments
folks to tag theme versions, e.g.tt1-blocks@0.4.3
).
Asked here: https://github.com/WordPress/theme-experiments/issues/195
Note: This is mainly about Site Editor, but it might also apply to other editors as well. Did anyone bump into issues while working on non-site-editor parts of Gutenberg?
Context
A few days ago I noticed we are depending on TT1B theme for a few PHP unit tests:
https://github.com/WordPress/gutenberg/blob/d944866a6dbf4bdb8b61979e111d772664a066a7/phpunit/class-block-templates-test.php#L16
https://github.com/WordPress/gutenberg/blob/e40a88cba8db618b5a479a6919c976d2f05e5371/phpunit/class-wp-rest-template-controller-test.php#L15
https://github.com/WordPress/gutenberg/blob/37e15797f2259ee0661b56b5a8f877bde9f29099/packages/e2e-tests/specs/performance/site-editor.test.js#L25
And a few more...
By default, WordPress tests use the default theme, which is the latest core theme. (twenty-twenty, TT1, etc.) So when we aren't using
switch_theme
then we are running tests against TT1 theme.The problem
We had a few changes in the site editor lately that we couldn't deliver without making changes in the TT1B theme. This means PRs can get blocked because we are depending on external dependencies (TT1B in this case). You can guess this lengthens the time it takes to deliver a PR since we need to wait for the changes in TT1B.
Running tests against the core theme has its own pro and con: pro: since we are running the test against the core theme, we can ensure that Gutenberg works with the core theme (or TT1B in case of site editor) con: if we want to make some changes, we need to coordinate it with the theme
Solution
A theme "fixture" that evolves with Gutenberg. Rather than using an external theme (TT1B), we should create a very minimal theme. We also gain a commit history where we can see what changes we had to make in this fixture theme to make it compatible with Gutenberg.
Implementation
We can create a
blocks-theme
directory in the repo and map it in.wp-env
Then we just need to implement the minimum to get the site editor working.