Skip to content

update regression tests and run them in CI tests#1761

Open
RamilCDISC wants to merge 2 commits into
mainfrom
regressionTests
Open

update regression tests and run them in CI tests#1761
RamilCDISC wants to merge 2 commits into
mainfrom
regressionTests

Conversation

@RamilCDISC

Copy link
Copy Markdown
Collaborator

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.

os.path.join("tests", "resources", "dictionaries", "meddra"),
"-r",
os.path.join("tests", "resources", "Rule-CG0027.json"),
"CORE-000237",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the rule from resources if we are no longer using it for the tests

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still being used in other tests, thats why I did not remove it.

"resources",
"CoreIssue1345",
),
"-r",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its the rule file actually has this

Core:
  Id: CDISC.SDTMIG.CG0019

So I think it is matching correctly the core id.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assertion will always be true. target_row[4] will never not be in target_row[4]

@SFJohnson24 SFJohnson24 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few questions--see comments

@gerrycampion gerrycampion linked an issue Jun 15, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression testing

2 participants