Skip to content

Conversation

kevinlacaille
Copy link
Contributor

@kevinlacaille kevinlacaille commented May 2, 2022

We simplified and clarified the planet --quiet and --verbosity tests as a teaching and learning exercise.

@kevinlacaille kevinlacaille added the CLI/SDK Interface Update the CLI and SDK to the finalized interface label May 2, 2022
@kevinlacaille kevinlacaille added this to the Data CLI MVP milestone May 2, 2022
@kevinlacaille kevinlacaille requested a review from sgillies May 2, 2022 17:50
@kevinlacaille kevinlacaille self-assigned this May 2, 2022
@kevinlacaille kevinlacaille linked an issue May 2, 2022 that may be closed by this pull request
@sgillies
Copy link
Contributor

sgillies commented May 2, 2022

The key things here @kevinlacaille @mkshah605 @jreiberkyle:

  • Asserting in "helper" functions or commands can improve clarity and eliminate the need for Python tricks. It requires catch_exceptions=False for CliRunner.invoke, otherwise the AssertionErrors aren't seen by pytest.
  • When patching, we need to patch the module that our unit under test is defined in.
  • Avoid shoehorning multiple tests into one function because a) that reduces readability and clarity, and b) we lose the ability to run the tests in random order. Tests that run in a fixed order are a great place for bugs to hide.
  • If you see if/else in a test function, that can be a sign that the test should be split up. There should be very little conditional logic within a test function.
  • If you see for loops in a test function, that can be a sign to use pytest parametrization instead.

@sgillies sgillies merged commit 8970cf6 into main May 2, 2022
@kevinlacaille kevinlacaille deleted the improve_cli_main_tests branch May 2, 2022 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI/SDK Interface Update the CLI and SDK to the finalized interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor quiet and verbosity tests

2 participants