Provide CI testing support #8
Conversation
mitchnegus
force-pushed
the
tests
branch
3 times, most recently
from
5339623 to
0565cc7
Compare
mitchnegus
force-pushed
the
tests
branch
2 times, most recently
from
c210aa1 to
56e1493
Compare
|
|
mitchnegus
force-pushed
the
tests
branch
2 times, most recently
from
9aba855 to
94b170f
Compare
| ] | ||
|
|
||
| [project.optional-dependencies] | ||
| test = [ | ||
| "tox<=4.23.2", |
There was a problem hiding this comment.
It looks like the dependency on tox was eliminated entirely. I think we should probably put it in the "format" or "dev" dependencies.
There was a problem hiding this comment.
I'll probably leave it in dev? We still have the ability for a user to do tox run or something like tox -e py311 which will run tests. This feels like a fair thing to leave as a developer capability (along with all the other linting/typing/etc. things that tox does).
| - name: Run unit tests | ||
| run: | | ||
| firewheel test unit -m 'not long and not mcs' \ | ||
| --cov --cov-report=term --cov-fail-under=60 |
There was a problem hiding this comment.
What was the reasoning for dropping this to 60?
There was a problem hiding this comment.
Whoops, I explained this in the extended body of the commit message because I expected you to ask 😁 (but then forgot to include it and elaborate in these notes).
The previous test level was not enough to get the unit tests to pass by themselves. Without a lot of digging, I think the reason is twofold:
- The previous coverage level included functional tests, which we can decide if we'd like to include or not. I don't know if we ever discussed that, but my gut feeling was that we'd want to know coverage for unit tests (i.e., are we testing the unit-level behavior of absolutely everything), while the value of the functional tests is at a higher level (i.e., does this stuff all work together).
- I've excluded tests that depend on other MCs or which take a while to run, meaning that any lines hit only by those are now uncovered. I think we actually do need to adjust that so that the long-running tests do get run at some point (although maybe we find a way to run them only on merges, not every push).
Add a dependency group for MC repos that may be installed along with the FIREWHEEL package. This commit also excludes tests relying on those MC repos from the GitHub testing workflow.
This drops coverage to 60% so that the unit tests pass. This coverage is lower than previous pipelines, likely because this coverage level only reflects unit tests (and not functional tests) and because the current pipeline is excluding both long-running tests and tests that depend on other MCs.
This provides a starting point for converting our GitLab CI testing infrastructure to the GitHub Actions workflow.
Workflow Upgrades
To avoid duplication in the workflow scripts, I've created a new set of composite actions to install minimega, discovery, FIREWHEEL, and tox. We should hopefully be able to use these composite actions to dramatically simplify some of the CI scripts we had before. I've also upgraded the versions of referenced reusable versions where possible.
Test Enhancements
The unit tests have been upgraded to provide increased utility and work better through the FIREWHEEL interface:
firewheel test unithelper now returns the appropriate exit code returned by pytest, which was previously unintentionally suppressed.-moption to select marked tests for inclusion/exclusion rather than adding custom options.Bug Fixes
This PR also fixes a few bugs in the current implementation:
test_cli_completion.pyscript).shlexfor proper parsing.