Closed jeanconn closed 3 years ago
Let's get #37 and https://github.com/sot/testr/pull/34 merged so we can automatically require that these only run on HEAD with skip.yml files.
Ha. I figured you'd prefer the failing tests as a fire under the other PRs.
With regard to the skip code, what is the advantage of naming them test_unit.py
vs test_unit_head.py
?
I have a PR in work that will make them pass on standalone platforms, so the name will be misleading. Basically the idea of selecting groups of tests from a test spec is going away, so just calling every unit test as test_unit.py
will be easier and more uniform.
See https://github.com/acisops/acis_thermal_check/pull/36. The test code here needs to set the ALLOW_NONSTANDARD_OFLSDIR
env var to anything. But note that this env var should only be set on non-HEAD platforms, because on the production runs that env var won't be set.
I would still say to only run on HEAD by default since the tests do take a while. Our smoke tests should be sufficient on non-HEAD platforms.
With regard to "I would still say to only run on HEAD by default ", does that mean you just want to skip them in skip.yml on other platforms? I've forgotten, with the skip.yml in place how would you then run them non-default on other platforms? Looks to me like the "include" doesn't override the skip?
Thus in the "by default" comment I'm confused if I need to add something to handle ALLOW_NONSTANDARD_OFLSDIR if I don't see a way to run that way in a non-default fashion here anyway.
Right, the ALLOW_NONSTANDARD_OFLSDIR
is not needed, sorry for that inconsistency in my thinking. At least for now let's only run this on HEAD. When needed it can be run manually on other platforms, and we can revisit defaults as desired.
Have you tested this on HEAD to show it gives the expected results? IIRC it should be testr.test()
not testr.testr()
. When you do so please put in a ## Functional testing
block in the description.
Good catch on the testr.test() vs testr.testr()! I did not notice, largely because all the tests had worked. I was planning to clean this PR up and ask for review again after getting that clarification, but will clean that test idiom up, rerun tests for the record, and mark the PR accordingly.
Oh wait, I'd forgotten, the convention in ska_testr is to use testr.testr() . It looks like this is consistent with the packages I spot checked just now and with the docs in testr . "This just calls the test()
function but includes defaults that are more appropriate for integrated package testing using run_testr" . Though maybe I knew that when I made this PR in December. I've forgotten.
Ah right, sorry for the noise.
Runs acis package unit tests via
testr.testr()
.After discussion we've set these to only run on head in ska_testr.
For functional testing, tests were run from a dev ska3-flight=2021.4rc2 environment; the acis unit tests ran and passed. To console it looked like there was some log handler overlap, as the number of messages from the acis tools sometimes seemed to increase over the session.
(when I ran the dea_check test today I had a transient fail due to what looks like a sqlite file IO issue).
Closes #38