Skip to content

Add acceptance test harness#927

Closed
BeigeBox wants to merge 3 commits intoEFForg:mainfrom
BeigeBox:feature/integration-tests
Closed

Add acceptance test harness#927
BeigeBox wants to merge 3 commits intoEFForg:mainfrom
BeigeBox:feature/integration-tests

Conversation

@BeigeBox
Copy link
Copy Markdown
Contributor

This is a set of tests to exercise the code against an actual device. It lets us catch regressions that other tests may not, as well as make it easier for devs to test that their code didn't cause any unintended side effects. The intent is to make it so that PRs are higher quality. Note the README.md it explains it a bit more detail.

Pull Request Checklist

  • The Rayhunter team has recently expressed interest in reviewing a PR for this.
    • If not, this PR may be closed due our limited resources and need to prioritize how we spend them.
  • Added or updated any documentation as needed to support the changes in this PR.
  • Code has been linted and run through cargo fmt.
  • If any new functionality has been added, unit tests were also added.
  • CONTRIBUTING.md has been read.

You must check one of:

  • No generative AI (including LLMs) tools were used to create this PR.
  • Generative AI was used to create this PR. I certify that I have read and understand the code, and that all comments and descriptions were authored by myself and are not the product of generative AI.

@untitaker
Copy link
Copy Markdown
Collaborator

I think this is a good idea, but I think we should split this testsuite up into multiple based on required capabilities. It should be possible to run a subset of these tests without a working /dev/diag, and therefore add that subset to CI. Alternatively we could try to mock /dev/diag and drive its output from tests.

@BeigeBox
Copy link
Copy Markdown
Contributor Author

I would be happy to do that, although I would also like to get buy in from the rest of the team before I spend more time working on this. It's been sitting for a month and if there's no interest from the folks doing the merging then it's not worth putting more time into it.

@cooperq
Copy link
Copy Markdown
Collaborator

cooperq commented Apr 16, 2026

I also think its worthwhile to have acceptance tests but this PR is too big to be reasonably reviewed. Would it be possible to commit the main wiring first and then the tests in smaller chunks?

@cooperq cooperq self-assigned this Apr 17, 2026
@cooperq
Copy link
Copy Markdown
Collaborator

cooperq commented Apr 22, 2026

Closing this for now but to be clear I really do want this code! I just want it in smaller chunks please!

@cooperq cooperq closed this Apr 22, 2026
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.

3 participants