Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HIL tests and workflow for card.binary #91

Closed
wants to merge 62 commits into from
Closed

Conversation

m-mcgowan
Copy link
Contributor

@m-mcgowan m-mcgowan commented Aug 19, 2023

Pushing for visibility, since this is 95% done.

Key points:

  • tests written using Unity, the default embedded test runner with PlatformIO

  • Extended unity in a way that allows test suites to be coded in individual files. Normally pio wants test suites to be in separate binaries.

  • Test matrices coded to avoid having to manually spell out all the permutations (interface, image, image size, send chunk size, read back chunk size.)

  • Test filtering to pinpoint failing tests.

  • Tests exercise Serial, I2C and AUX serial at various baudrates.

  • BinaryGenerator abstraction allows real binary images and generated data to be used in tests.

  • Workflow complies and uploads firmware to Swan after pulling in note-c from this repo, replacing the version in note-arduino pio dependencies.

  • Handlers used after each time the binary buffer is filled include none (verify-only), web.put/post (one hit to the endpoint or in chunks), and note.add.

  • Runs tests and captures test report, publishes it as part of the Github Checks with code annotations for failures.

  • Code structured so that it can be made a stand-alone application for easier one-off testing.

  • External verification is provided by a MD5 server. This saves received content as well as returning the md5 of each piece of content. It can also serve as the endpoint for note.add with binary data, allowing the data to be later verified with web.get to the same endpoint.

  • The workflow sets up the md5 server locally, sets up a local tunnel, creates notehub proxy and http routes to the md5 server endpoint and runs the tests against these.

Currently only a simple smoke test is present for quick results while testing the workflow in Github.

TODO:

  • expand test suite with all the tests with compile time options for just smoke tests vs weekly extensive testing
  • Implement alternative tunnel service since localtunnel seems unreliable.

Copy link
Collaborator

@zfields zfields left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool set of tests; very tidy and organized. Well done!

--junit-output-path test.xml \
--project-dir "$PIO_PROJECT_DIR"
- name: Publish Test Report
uses: mikepenz/action-junit-report@v3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we trust "mikepenz"? Are there any opensource projects, or any other more official options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very valid point, and something I check before using any 3rd party actions. It is open source. The source code is available at https://github.com/mikepenz/action-junit-report, the latest tag, which we are using, its at https://github.com/mikepenz/action-junit-report/tree/v3.8.0/src. I didn't find anything more "official", i.e. from Github, but of course will continue looking.

I've looked through the sources for this action, and see no threat. We can discuss internally what a breach of trust would mean for any of the actions we are using.

size_t chunkRead = image.read(buffer, thisChunk);
assert(chunkRead == thisChunk);

for (size_t i=0; i<chunkRead; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be especially interesting in the fail case, where the MD5 sum did not match. Although we don't have a guaranteed error message that would indicate this was the precise point of failure.

It would be cool to see how far it went off the rails and starting at which index.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure at present how we can fake a bad MD5 checksum. Perhaps by overriding the interface communications hooks passed to note-c so we can deliberately inject bad data?

-Wall
-Werror # ensure unused functions in the test script generate an error
-D PIO_FRAMEWORK_ARDUINO_ENABLE_CDC
'-D PRODUCT_UID="com.blues.mat:test"'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you iron out the bugs, get Brandon (or someone with special powers) to spin up a com.blues.test project for your tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. For now I'm playing in my own sandbox. This covers all the test runners and their associated assets such as Notehub projects. Longer term, I want to move all the runners to a Runners Group at organization level and will need some powers to be able to make that happen. We can then move all the notehub projects to a company account so they are no longer on my personal account.

@m-mcgowan m-mcgowan force-pushed the mdm-card-binary-test branch 2 times, most recently from 8aa6caf to 1609393 Compare September 8, 2023 08:20
@m-mcgowan m-mcgowan marked this pull request as ready for review September 8, 2023 08:56
```
{"req":"web.get","route":"card.binary.6126291556","name":"Random_1234?chunk=3","content":"application/octet-stream","binary":true,"crc":"0025:AD91438A"}
{"result":504,"body":{"err":"wire: no response from service {service}: socket read {network}: network timeout {network-timeout} {network}"}}
```
…lly switched between a local dev build and CI build.
# Conflicts:
#	n_helpers.c
#	note.h
#	test/src/NoteBinaryDecode_test.cpp
#	test/src/NoteBinaryEncode_test.cpp
#	test/src/NoteBinaryStoreReceive_test.cpp
#	test/src/NoteBinaryStoreTransmit_test.cpp
@haydenroche5
Copy link
Collaborator

Migrated to #124.

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