Skip to content

Workflows: Try different SHA #70048

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 2 commits into from
May 4, 2025
Merged

Workflows: Try different SHA #70048

merged 2 commits into from
May 4, 2025

Conversation

@Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented May 3, 2025

What?

Related #70023.

Try different SHA for the performance tests workflow. The current choice might have been too recent, and I don't see new commits in https://www.codevitals.run/project/gutenberg.

How?

See https://developer.wordpress.org/block-editor/explanations/architecture/performance/#update-the-reference-commit.

Testing Instructions

These changes are hard to test without merging.

@Mamaduka Mamaduka added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label May 3, 2025
@Mamaduka Mamaduka requested a review from desrosj as a code owner May 3, 2025 07:56
@Mamaduka Mamaduka requested a review from t-hamano May 3, 2025 07:56
@github-actions
Copy link

github-actions bot commented May 3, 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: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>

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

@t-hamano
Copy link
Contributor

t-hamano commented May 3, 2025

From what I can see, it looks like a request error is occurring here:

gutenberg/bin/log-performance-results.js

Lines 88 to 94 in bcc41b1

const req = https.request( options, ( res ) => {
console.log( `statusCode: ${ res.statusCode }` );
res.on( 'data', ( d ) => {
process.stdout.write( d );
} );
} );

Normally, a successful HTTP request to the code health should get a response with a 200 status code:

image

However, after updating the base commit, we are getting a 405 error, which means the request is not allowed for the resource:

image

Is this issue related to #69126? Maybe adding the appropriate permissions to that job will solve it?

@Mamaduka
Copy link
Member Author

Mamaduka commented May 3, 2025

Is this issue related to #69126? Maybe adding the appropriate permissions to that job will solve it?

I can see commits tracked in CodeVitals after this PR, but double-checking won't hurt.

cc @johnbillion, @desrosj

@t-hamano
Copy link
Contributor

t-hamano commented May 3, 2025

One more thing I was wondering, does this SHA need to be updated?

gutenberg/.github/workflows/performance.yml

Line 112 in bcc41b1

./bin/log-performance-results.js "$CODEHEALTH_PROJECT_TOKEN" trunk "$GITHUB_SHA" c7722262e65a3f4d0f1a2d1ad29eccb2069509e4 "$COMMITTED_AT"

It appears that in the past, when the minimum required WordPress version has been bumped, this commit hash has been updated as well.

I noticed that #70023 didn't update this line 🤔

@Mamaduka
Copy link
Member Author

Mamaduka commented May 3, 2025

Great catch, @t-hamano! I did a search and replace, but it looks like I missed the spot.

t-hamano
t-hamano approved these changes May 3, 2025
View reviewed changes
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give it a try 👍

@Mamaduka Mamaduka merged commit 82cd098 into trunk May 4, 2025
61 checks passed
@Mamaduka Mamaduka deleted the fix/performance-tests-try-different-sha branch May 4, 2025 07:02
@github-actions github-actions bot added this to the Gutenberg 20.8 milestone May 4, 2025
@Mamaduka
Copy link
Member Author

Mamaduka commented May 4, 2025

This worked. Thanks for catching the missing SHA replacement, @t-hamano!

chriszarate pushed a commit to chriszarate/gutenberg that referenced this pull request Jul 1, 2025
@Mamaduka @t-hamano @chriszarate
Workflows: Try different SHA (WordPress#70048)
b2e01b1 
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t-hamano t-hamano t-hamano approved these changes

@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 20.8

Development

Successfully merging this pull request may close these issues.

3 participants

@Mamaduka @t-hamano