update regression tests and run them in CI tests#1761
Conversation
| os.path.join("tests", "resources", "dictionaries", "meddra"), | ||
| "-r", | ||
| os.path.join("tests", "resources", "Rule-CG0027.json"), | ||
| "CORE-000237", |
There was a problem hiding this comment.
can we remove the rule from resources if we are no longer using it for the tests
There was a problem hiding this comment.
It is still being used in other tests, thats why I did not remove it.
| "resources", | ||
| "CoreIssue1345", | ||
| ), | ||
| "-r", |
There was a problem hiding this comment.
I am a bit confused on this as -r is only for published rules and this is not a CORE-ID. Is CORE silently ignoring the -r and executing and that is what is being testing?
There was a problem hiding this comment.
Its the rule file actually has this
Core:
Id: CDISC.SDTMIG.CG0019
So I think it is matching correctly the core id.
There was a problem hiding this comment.
yes but why is the -r there if it executing from the local rule file? Is it not running the rule specified with -r purposefully? Trying to understand that line specifically as it relates to the test
| "-s", | ||
| "sdtmig", | ||
| "-v", | ||
| "3-4", |
There was a problem hiding this comment.
this test seemed to want a version of SDTMIG that doesnt contain a rule and was rewritten to not do that --was there a reason for this?
There was a problem hiding this comment.
Yes, I forgot to update that part. The real intention was to discuss this test. CoreIssue1487 required that we skip a rule if that is not part of the selected version. Now the engine raises.
cdisc_rules_engine.exceptions.custom_exceptions.LibraryMetadataNotFoundError: No library metadata found for standard 'sdtmig' version '5.0'.
We can either update the test to confirm for this exception or do we want to restore engine's functionality to skip instead of an exception?
There was a problem hiding this comment.
if you are running for an invalid standard, i would expect an exception
| break | ||
| assert target_row, "Rule CORE-000354 not present in 'Rules Report' sheet." | ||
| assert ( | ||
| target_row[4] in target_row[4] |
There was a problem hiding this comment.
this assertion will always be true. target_row[4] will never not be in target_row[4]
SFJohnson24
left a comment
There was a problem hiding this comment.
a few questions--see comments
The PR updates the broken regression tests and removes the regression markers from all the tests to ensure all QARegressionTests are run during the CI pipeline.