-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wp-env.json: Pin tt1-blocks dependency to v0.4.3 #28741
Conversation
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
Well, that didn't work 😕 |
I think we probably need the long-form SHA (unless we get tags), but that's a bit gross -- ideally, we could pass shortened ones, and they'd get auto-expanded for us. |
Found something here: nodegit/nodegit#569, will try that. |
Hmm, maybe the problem isn't so much in expanding short SHAs, but in the branch fetching logic (that maybe doesn't work with commit SHAs)? Found this: https://github.com/nodegit/nodegit/blob/7df154a5eb9e93ad933f0a9864f06e2395d32d21/examples/checkout-remote-branch.js -- might give this a try tomorrow. |
I think the underlying issue is that I've noticed that for the GB release tools, we use a different git library, gutenberg/bin/plugin/lib/git.js Line 4 in 8d1d96d
OTOH, nodegit is only used by wp-env 's download script.
So I think that he fact that |
I just discovered #26660. I think it's fair to say that this PR is blocked by that issue. |
@carolinan has kindly tagged |
Any idea how we'll keep this up to date? |
Thanks for approving, Riad! I'm almost surprised this works now -- it never did when I was using a commit SHA to pin instead. I guess it's because of the way we fetch with This means this PR is no longer blocked by #26660. However, it might still be nice to get #28848 in, since
Manually, I'm afraid. That's the price we pay for a stable test environment. The alternative would be to stop relying on an external theme for testing, as discussed in #28692 -- but that has of course other drawbacks; most of all, it's no longer a "real world" theme, so we might lose touch with latests developments and requirements of block template themes more easily. |
Hmm, |
cd9561f
to
703c24a
Compare
#28884 has been merged, and I've rebased this PR 🤞 |
Description
Gutenberg's end-to-end tests were broken for a while after TT1 Blocks removed their
front-page
template (WordPress/theme-experiments#179) -- which our e2e tests relied on. This was thankfully fixed on the Gutenberg side by @Addison-Stavlo in #28638.Per discussion with @jeyip, we might be able to avoid a similar situation in the future by pinning our TT1 Blocks dependency to a given version.
See #28692 for more background.
How has this been tested?
Verify that automated tests pass.