Page MenuHomePhabricator

[Goal] Increase stability and decrease tech debt of visual regression tool
Closed, ResolvedPublic

Description

Goal definition

Specific: What do we want to achieve?

Our visual regression tool (Pixel) is more stable and has less tech debt as defined by the criteria below.

Measurable: How will we know when we've reached our goal?

The following tasks and objectives have been completed preferably before I leave for sabbatical on July 5:

  • Pixel is versioned (T307936 in progress) (See T307936#8020407)
  • The database can easily be reset on each test run by passing a flag (e.g. --reset-db) (Planning to include this in T307936 in progress). This should make it easier to add tests that change the state of the database.
  • Add visual regression tests for the typeahead search component T306846 . The typeahead search component is one of the most frequently occurring visual regressions and the tests will also be helpful when the wvui implementation is replaced with the codex implementation.
  • Revise/fix visual regression checkout logic and increase unit test code coverage T309739
  • Pixel's report on https://pixel.wmcloud.org/ and cron job should survive the Wikimedia Cloud VPS server restarting. Currently, I believe manual intervention is required to start es-dev-server and screen as described in T305563#7879426 . Can we point the apache server that is already setup at the report's static directory and remove es-dev-server?
  • Also, the Cloud VPS instance volume has limited space and /srv/pixel.log file currently grows without limits. Can we install logrotate?
  • The regression tests are consistently deterministic. We are getting close to this, but I've noticed one or two of the tests (e.g. MediaWiki_Test_sticky_header_vector-2022_logged-in_scroll_0_viewport_2_desktop) are sometimes flaky. Running all tests (desktop, mobile, echo, and forthcoming search tests) 10 times against the same code should consistently pass. See T309742#8045516
  • The CI job that currently only runs the desktop tests for each PR should be changed to run all tests and should block merging into the main branch if they fail. See https://github.com/wikimedia/pixel/pull/85
  • Github/npm ownership is transferred from nicholasray to wikimedia (T306731). If this proves too difficult or time consuming, at least ensure that each member of the team has the access they need to the repos.

Achievable: What support will we need to achieve our goal?

Support from other teams is most likely not needed except maybe for the last step (I'm not exactly sure what goes into the wikimedia github ownership process). Apart from that, dedicated time and space will be required to complete the above tasks within a month.

Longer term outlook

Although we won't have time to experimentally add Pixel to Vector's CI pipeline T308194 this month, completing the above tasks and improving the stability of Pixel will help facilitate that work.

Event Timeline

@Jdlrobson @ovasileva As requested, here is the new ticket for the remaining visual regression tool work that would ideally be completed before I leave for sabbatical

The CI job that currently only runs the desktop tests for each PR should be changed to run all tests and should block merging into the main branch if they fail.

Is this one within our control or do we need help from RelEng? I'd rather not take this on if we need their help.

The CI job that currently only runs the desktop tests for each PR should be changed to run all tests and should block merging into the main branch if they fail.

Is this one within our control or do we need help from RelEng? I'd rather not take this on if we need their help.

It's within our control and doesn't need RelEng. I setup a github action at https://github.com/nicholasray/pixel/blob/main/.github/workflows/push.yml that runs for each pull request to Pixel but it only runs for desktop. It just needs to be edited to include the other tests as well

Nice. I chatted with @ovasileva yesterday and she's happy for us to work on this for the next month. I think the work is well scoped and achievable in that time so thank you for taking the time to write it out!
Regarding " Github/npm ownership is transferred from nicholasray to wikimedia" I think we can resolve this ticket even without completing this if necessary if it drags out.

xel's report on https://pixel.wmcloud.org/ and cron job should survive the Wikimedia Cloud VPS server restarting. Currently, I believe manual intervention is required to start es-dev-server and screen as described in T305563#7879426 . Can we point the apache server that is already setup at the report's static directory and remove es-dev-server?

This is already done so marking.

xel's report on https://pixel.wmcloud.org/ and cron job should survive the Wikimedia Cloud VPS server restarting. Currently, I believe manual intervention is required to start es-dev-server and screen as described in T305563#7879426 . Can we point the apache server that is already setup at the report's static directory and remove es-dev-server?

This is already done so marking.

Awesome, thanks!

  • The regression tests are consistently deterministic. We are getting close to this, but I've noticed one or two of the tests (e.g. MediaWiki_Test_sticky_header_vector-2022_logged-in_scroll_0_viewport_2_desktop) are sometimes flaky. Running all tests (desktop, mobile, echo, and forthcoming search tests) 10 times against the same code should consistently pass.

I've added some logic at the end of the onReady.js files which seems to help improve the deterministic nature of the tests. Although I think they can still sometime flake out (e.g. the logo image request can sometimes fail for some reason), I was able to run ./pixel.js reference followed by looping through ./pixel.js test 100 times for the desktop, echo, and mobile test groups and have them all pass. Therefore, I'm marking this as complete.

  • The CI job that currently only runs the desktop tests for each PR should be changed to run all tests and should block merging into the main branch if they fail.

If it's helpful (not sure it is), I've add a PR for this at
https://github.com/wikimedia/pixel/pull/85

Feel free to discard though, I don't have a strong preference either way! In the meantime, I'm marking this a complete

nray claimed this task.

Marking this as done! Thank you everyone for your help!