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

small integration test improvements. #194

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Commits on Mar 7, 2023

  1. chore: avoid fixed sleep in integration tests.

    Prior to this commit the `tests/tests.rs` integration tests had an
    unconditional 1 second sleep waiting for a server to become available.
    This is slightly problematic:
    
    * Waiting a full second if the server becomes available sooner is
      unnecessarily slow.
    * Conversely, we can't be sure the server is available after the 1s
      sleep.
    
    This commit replaces the sleeps with a loop of up to 10 iterations that
    tries to connect to the server port. If any iteration succeeds we return
    immediately to avoid unnecessary waiting. If an iteration fails, we wait
    longer, increasing the duration each time we wait (up to a maximum of 10
    iteration).
    cpu committed Mar 7, 2023
    Configuration menu
    Copy the full SHA
    9531fbc View commit details
    Browse the repository at this point in the history
  2. fix: kill server before asserting on output.

    Prior to this commit the `server()` test would call `assert_eq!` to
    assert on client output matching expected _before_ calling `srv.kill()`
    on the `Child` spawned by `server_command()`.
    
    The upstream rustdocs for `std.process.Child` mention:
    > There is no implementation of Drop for child processes, so if you do
    > not ensure the Child has exited then it will continue to run, even
    > after the Child handle to the child process has gone out of scope.
    
    The net result is that if the assertion in `server()` fails, we leave
    a running server instance behind. This manifests as a port conflict when
    re-running the test without first killing the orphan.
    
    This commit moves the `srv.kill()` call to before the `assert_eq`. This
    avoids leaking the server process if the assertion fails.
    cpu committed Mar 7, 2023
    Configuration menu
    Copy the full SHA
    2b7da58 View commit details
    Browse the repository at this point in the history
  3. chore: improve server integration test output.

    This commit adds a few quality of life improvements to the server
    integration test:
    
    1. The `--silent` curl arg is removed so that we can get diagnostic info
       from curl on stderr in the event of a failure. Our test only asserts
       on stdout content, which remains unaffected by this change.
    2. If the `curl` command invocation status isn't success, we print the
       stderr content enabled by not using `--silent`. This will provide
       more diagnostic context to investigate the failure.
    3. The `assert_eq` is changed to assert on a string value created from
       stdout instead of raw bytes. This is easier for humans to diff in the
       test output compared to a slice of byte values.
    
    Combined these changes should make it easier to diagnose future test
    failures without needing to iterate in CI.
    cpu committed Mar 7, 2023
    Configuration menu
    Copy the full SHA
    fd8a111 View commit details
    Browse the repository at this point in the history

Commits on Mar 8, 2023

  1. chore: output curl version in server test failure.

    In the event the `server()` integration test fails it's helpful to know
    the version of `curl` that was used. This commit includes this
    information when the `curl` client invocation returns a non-zero status
    code.
    cpu committed Mar 8, 2023
    Configuration menu
    Copy the full SHA
    ffa7677 View commit details
    Browse the repository at this point in the history