Fix VRT test runner in CI.

[techanvil]

Bug Description

At present the VRT workflow is not actually running the tests, and reports a false positive when it runs.

We need to fix this to ensure the tests run, and fix any issues with the tests themselves that emerge as a result of not having run them for a while.

Steps to reproduce

1. Go to any recent VRT workflow run, e.g. https://github.com/google/site-kit-wp/actions/runs/3470334983/jobs/5798409684#step:5:268
2. Look at the end of the log for the Run Backstopjs step.
3. Note how after building the Docker image, it logs out the input device is not a TTY and then immediately after logs To see the VRT report indicating an error attempting to execute the test run.
4. As a sanity check here is a PR which introduces a deliberate VRT error which should fail but does not: Verify expected VRT failure. #6150

Screenshots

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• The VRT CI workflow should run the tests, and succeed/fail based on the status of the test run.
• Once the workflow is running properly, the state of play of the VRT test suite should be assessed. Issues should be added as necessary to follow up with any remedial work.
  • Bear in mind the existing issue Resolve VRT issues for local runs #5824 which may need to be updated or closed depending on the outcome of the above.

Implementation Brief

• To fix the the input device is not a TTY, remove the -i flag from the docker run command in bin/backstop.
• The bin/backstop script will need further tweaking in order to ensure the CI step fails if the docker run command fails:
  • Capture the status of the docker run command and exit with a 0 for success or 1 for failure at the end of the file.
• Ensure the VRT test suite still runs locally.
• As per the AC, create followup issues as necessary to fix any failures.

Test Coverage

• No new tests needed.

QA Brief

QA:Eng:

Note for code reviewers: These steps will only be possible to verify on google/site-kit-wp once the PR has been merged to develop. However they can be seen in action on this fork of the repo: techanvil/site-kit-wp, with code changes evident here and a sample test run here.

• Verify the VRT tests are running in CI.
• Verify that the VRT workflow fails when the tests fail. We'll have to wait for the tests to be fixed in the followup issue for the success verification.
• Verify that a followup issue has been created as per the AC: Fix the VRT tests that are failing in CI. #6185

Changelog entry

• N/A

[aaemnnosttv]

• Capture the status of the docker run command and exit with a 0 for success or 1 for failure at the end of the file.

@techanvil would set -e be sufficient here?

Otherwise this looks good. Please add an estimate and then this is good to go 👍

[techanvil]

Thanks @aaemnnosttv. -e would not be sufficient as we still want to echo the report location when the test run fails:

site-kit-wp/bin/backstop

Lines 28 to 34 in 597e8cb

	docker run --rm -it --mount type=bind,source="$ROOT_DIR",target="/src" $IMAGE $1 --config=/src/tests/backstop/config.js
	# Display a link to the report only after the "test" command.
	if [ $1 == "test" ]; then
	echo ""
	echo "To see the VRT report open this URL in your browser: file://$ROOT_DIR/tests/backstop/html_report/index.html"
	fi

I've estimated it as a 7, hopefully erring on the side of caution.

[aaemnnosttv]

-e would not be sufficient as we still want to echo the report location when the test run fails

@techanvil ah yes, of course. SGTM, thanks!

[tofumatt]

QA here was done as part of the Code Review, so no further QA needed, so moving this directly to Approval.

[aaemnnosttv]

VRT is running in CI again but failing (which was expected here). These will be fixed in #6185.