Page MenuHomePhabricator

Revise visual regression checkout logic
Closed, ResolvedPublic3 Estimated Story Points

Description

Currently, when ./pixel.js reference or ./pixel.js test is run, main.js delegates to checkoutBranch.js and checkoutPatch.js which perform the following ordered steps:

  1. The correct branch is checked out (main by default or the brach specified by the -b flag)
  2. composer install or composer update is run
  3. php maintenance/update.php is run
  4. The revision (if any) and its dependencies (if any) are fetched from gerrit and rebased on top of the correct branch

However, this logic currently incorrectly assumes that the patch (if any) will not have any composer or database changes as neither composer update or the update.php script is run after the patch is checked out.

Instead, the logic should look like the following:

  1. Checkout the patch (if any) and rebase on the correct branch
  2. composer install or update is run
  3. php maintenance/update.php is run

Acceptance Criteria

  • The logic in checkoutBranch.js and checkoutPatch.js are revised to execute the steps listed above. Consider merging both of these into one class
  • Since this logic is significant to the function of the visual regression tests, add unit tests with code coverage > 90%

Event Timeline

PR at https://github.com/nicholasray/pixel/pull/58

After this commit, I have a couple other commits which I'm hoping will resolve some of the remaining tasks on T309742

cjming subscribed.