Skip to content

CodeVitals: Use a different SHA for the base branch #70565

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2025
Merged

CodeVitals: Use a different SHA for the base branch #70565

merged 1 commit into from
Jun 30, 2025

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Jun 30, 2025 •
edited
Loading

Follow-up #70553

What?

This PR updates the commit hash of the base branch used by Code Vitals, which should resolve the failing performance tests in the trunk branch.

Why?

#70553 fixed the npm library inconsistency, npm ci now works properly, and some performance tests now work properly.

Meanwhile, on the trunk branch, we have performance tests that run when the event is "push", i.e. when a PR is merged. Those performance tests use the 7f6a627 commit hash, but that commit doesn't yet contain #70553, so npm ci fails. As a result, CodeVitals also doesn't update correctly.

How?

Update the commit hash with #70553 (1b8efa7).

This should allow npm ci to succeed on the base branch. However, this commit hash is usually only updated when we update the minimum WordPress version that the Gutenberg plugin supports. Therefore, I am concerned that updating the commit hash now will affect some metrics.

Testing Instructions

This PR is unfortunately not testable as it tries to resolve performance tests that run only on trunk. Hopefully, the performance tests will pass on the trunk branch, and CodeVitals will be updated.

@t-hamano t-hamano added Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Performance Related to performance efforts labels Jun 30, 2025
@Mamaduka Mamaduka added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. and removed Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Performance Related to performance efforts labels Jun 30, 2025
@Mamaduka Mamaduka requested review from Mamaduka and youknowriad June 30, 2025 09:05
@t-hamano t-hamano marked this pull request as ready for review June 30, 2025 09:09
@t-hamano t-hamano requested a review from desrosj as a code owner June 30, 2025 09:09
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 30, 2025 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@youknowriad
Copy link
Contributor

The problem with this change is that it will fix the issue but since the chosen hash doesn't have any value in the code vitals dataviews, we will see "jumps" in the graph for all the metrics because we don't have the reference values to compare to.

@youknowriad
Copy link
Contributor

If I'm understand the issue though, it seems we don't have any potential reference commit where the tests pass right? Which means we don't have much choice right?

@Mamaduka
Copy link
Member

If I'm understand the issue though, it seems we don't have any potential reference commit where the tests pass right? Which means we don't have much choice right?

I think that's true. All previous SHA seem to be affected by the build error.

@youknowriad
Copy link
Contributor

I'm ok just accepting the jumps personally. But folks monitoring the graphs need to be aware about it.

@t-hamano
Copy link
Contributor Author

Thank you everyone for your reviews.

If I'm understand the issue though, it seems we don't have any potential reference commit where the tests pass right? Which means we don't have much choice right?

I think that's true. All previous SHA seem to be affected by the build error.

This is true. Let's merge this PR and hope the performance tests pass.

@t-hamano t-hamano merged commit b308bc8 into trunk Jun 30, 2025
85 of 88 checks passed
@t-hamano t-hamano deleted the fix-perf-test-sha branch June 30, 2025 12:30
@github-actions github-actions bot added this to the Gutenberg 21.2 milestone Jun 30, 2025
@t-hamano
Copy link
Contributor Author

All CI passed on the trunk branch, but there was a huge change in the CodeVitals graph 😂

https://www.codevitals.run/project/gutenberg/firstBlock

image

@youknowriad
Copy link
Contributor

It is not unexpected for me. Using a commit that has no previous value basically mean starting the graphs from scratch

@Mamaduka
Copy link
Member

@t-hamano, it might be worth sharing the reason for this stats drop in the Slack channel(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad approved these changes

@Mamaduka Mamaduka Awaiting requested review from Mamaduka

@desrosj desrosj Awaiting requested review from desrosj desrosj is a code owner

Assignees
No one assigned
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Milestone
Gutenberg 21.2
Development

Successfully merging this pull request may close these issues.
3 participants
@t-hamano @youknowriad @Mamaduka