Skip to content

Conversation

nicholasbishop
Copy link
Member

@nicholasbishop nicholasbishop commented Nov 23, 2022

This contains a number of related improvements to the test runner. The main changes are:

  • If the screenshot test fails for some reason, don't dump the raw pixel data to the console. The data is not human-readable anyway, and if your console is slow it can take a while to print it all.
  • Drop the second serial device from the test runner. We added this a while ago to fix the conflict in the screenshot test between needing to open the serial device in exclusive mode (so that the timeout and input aren't messed with by other EDK2 code) and needing to continue writing logs after the screenshot. Turns out there's a simpler way to fix this: we can use a single serial device, which does break the console logger, but we can restore the console output with connect_controller.
  • Add TESTS_COMPLETE message that's sent the same way as the SCREENSHOT message. This is used to signal at the end of the tests that they completed successfully. If the host never receives that message, it exits with an error.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@nicholasbishop nicholasbishop force-pushed the bishop-single-serial-device branch from 8614241 to a869811 Compare November 23, 2022 02:20
@nicholasbishop nicholasbishop marked this pull request as draft November 23, 2022 03:02
@nicholasbishop nicholasbishop force-pushed the bishop-single-serial-device branch from a869811 to 0752e84 Compare November 23, 2022 03:20
@nicholasbishop nicholasbishop marked this pull request as ready for review November 23, 2022 03:26
Use `Box<dyn ...>` instead. No functional change, just reduces the need
to add generic parameters outside of the `Io` implementation.
Use `assert!` rather than `assert_eq!` so that if the screenshot
comparison fails, we don't dump all the raw pixel data to the console.
The screenshot test sends a command to the host via the serial device,
then waits for a reply before continuing. We used a second serial device
just for this request and reply so that we could open the protocol in
exclusive mode without breaking logging for tests after that.

It turns out there's a simpler way to do this: we can use the same
serial device as the one used for logs, and after we're finished use
`connect_controller` to restore the connection between the serial device
and the console output.
At the end of the test-runner app, right before we exit boot services,
send a TestsComplete message to the host. If the host doesn't receive
this message, the tests will be marked as failed.
@phip1611 phip1611 force-pushed the bishop-single-serial-device branch from 0752e84 to c05a9e0 Compare November 24, 2022 09:13
@nicholasbishop nicholasbishop merged commit 3c03e45 into rust-osdev:main Nov 24, 2022
@nicholasbishop nicholasbishop deleted the bishop-single-serial-device branch November 24, 2022 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants