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

Streamline integration tests and add first test for read timeouts #184

Merged
merged 5 commits into from
May 13, 2024

Conversation

sirhcel
Copy link
Contributor

@sirhcel sirhcel commented May 6, 2024

This PR cleans up the following points:

  • No longer requiring the the dummy environment variables from Clean up integration tests #181 (they are evaluated at runtime on demand now)
  • Running tests for the SerialPort trait on all platforms

It also adds the first test for read timeouts showing the blocking behavior for a timeout of zero as described in #79 and paves the way for integrating #79.

This cleans up the initial attempt of specifying the hardware ports to
be used for testing by:

    * Requiring the configuration environment variables only to be
      present when actually required by a test case

    * Evaluating these environment variables at runtime, eliminating the
      footgun that changing them does not trigger rebuilding the tests

    * No longer requiring to set dummy configuration variables when not
      running any hardware tests at all (like in CI).

The configuration environment variable prefix changes from
SERIALPORT_RS_TEST_ to SERIALPORT_TEST_ to match the crate name and not
the repository name.

The changes add two new dev dependencies which both look worth it:

    * The envconfig crate eliminates any need for custom error reporting
      about which environment variable is missing (which not comes for
      free when just using std::env::var)

    * The rstest crate conveniently allows to use test fixtures and
      supports parametrizing tests which will likely be used by the next
      test cases to come
The expected behavior is the same for all platforms. Let's run these
test cases everywhere then.
@sirhcel sirhcel requested a review from eldruin May 6, 2024 19:53
This apparently does not reduce noise in CI but at least allows to build
the examples with Rust 1.59.0.
Copy link
Contributor

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@eldruin eldruin merged commit c320919 into serialport:main May 13, 2024
30 checks passed
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.

None yet

2 participants