Skip to content
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

Fix release performance tests #50699

Merged
merged 1 commit into from
May 17, 2023
Merged

Fix release performance tests #50699

merged 1 commit into from
May 17, 2023

Conversation

WunderBart
Copy link
Member

@WunderBart WunderBart commented May 17, 2023

What?

The disparity between the performance.js runner source and the tests' source is causing the release perf tests to fail. The error is that the tests are saving result artifacts under a different name than the perf script is expecting them to be, so it cannot find them. For example, we can look at the most recent release perf job. The command is:

./bin/plugin/cli.js perf "wp/$WP_MAJOR" "$PREVIOUS_RELEASE_BRANCH" "$CURRENT_RELEASE_BRANCH" --wp-version "$WP_MAJOR"

...which values for that run are:

./bin/plugin/cli.js perf wp/6.2 release/15.7 release/15.8 --wp-version 6.2
  • The source of the workflow and the performance.js script is the branch that the job is dispatched from, which is actually a tag ref tags/v15.8.0-rc.1.
  • The source of the tests is the first branch to be compared, which is wp/6.2, hence the disparity.

How?

The fix is to make the tests source the same as the runner source (GITHUB_SHA).

Testing Instructions

The next release perf tests should pass.

@WunderBart WunderBart requested a review from youknowriad May 17, 2023 14:21
@WunderBart WunderBart self-assigned this May 17, 2023
@WunderBart WunderBart added the [Type] Performance Related to performance efforts label May 17, 2023
@youknowriad
Copy link
Contributor

Yes, this changes makes a lot of sense to me (at least until the runner becomes just a runner and loses any knowledge of the metrics)

@WunderBart WunderBart enabled auto-merge (squash) May 17, 2023 14:33
@WunderBart WunderBart merged commit e312f9c into trunk May 17, 2023
@WunderBart WunderBart deleted the fix/perf-release-tests branch May 17, 2023 14:54
@github-actions github-actions bot added this to the Gutenberg 15.9 milestone May 17, 2023
@WunderBart
Copy link
Member Author

WunderBart commented May 25, 2023

@youknowriad the fix worked, but the tests still failed. This time there seems to be an actual test failure:

From https://github.com/WordPress/gutenberg/actions/runs/5074051713/jobs/9113812136:

FAIL packages/e2e-tests/specs/performance/site-editor.test.js (97.463 s)
  Site Editor Performance
    ✓ Loading (1 of 4) (15281 ms)
    ✓ Loading (2 of 4) (15407 ms)
    ✓ Loading (3 of 4) (15057 ms)
    ✓ Loading (4 of 4) (2440 ms)
    ✕ Typing (22077 ms)

  ● Site Editor Performance › Typing

    Couldn't find "Paragraph" in the Blocks category.

      at insertFromGlobalInserter (../e2e-test-utils/build/@wordpress/e2e-test-utils/src/inserter.js:254:9)
          at runMicrotasks (<anonymous>)
      at insertBlock (../e2e-test-utils/build/@wordpress/e2e-test-utils/src/inserter.js:330:2)
      at Object.<anonymous> (specs/performance/site-editor.test.js:155:3)

The above is failing only for the wp/6.2 branch. This is what the failure looks like:

Typing 2023-05-24T23-13-55

Let me know if it rings a bell. I'm going to investigate this in a follow-up PR.

/cc @kevin940726

@youknowriad
Copy link
Contributor

What seems weird is that the "list" block seems to be selected which is probably why the paragraph is not shown in the inserter. So the question that needs solving is why that list block is selected?

Also, it seems that the list block is in the "center" of the canvas. So potentially we might have have a step that says "click canvas" or "click editor" or something like that, that is selecting the list block inadvertently.

I think we do have a step to "click the canvas" to enter "edit mode" in the site editor (in branches where the browse mode is available) but that click shouldn't select any block, it should just switch to "edit mode".

@kevin940726
Copy link
Member

Seems oddly similar to #48208 (comment).

@WunderBart
Copy link
Member Author

WunderBart commented May 25, 2023

Seems oddly similar to #48208 (comment).

Huh, forgot about this one - it looks exactly the same! 😬 I'll try debugging the current issue, but I was also about to start migrating perf specs to Playwright - might be worth checking first if Playwright does a better job OOTB in terms of stability here.

@WunderBart
Copy link
Member Author

Alright, it passed after a rerun. 🎉 I'm going to focus on Playwright migration, then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants