Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule test engine bug fix - sometimes fail to flag not_applicable and undetermined #1422

Merged
merged 116 commits into from
Aug 13, 2024

Conversation

jugonzal07
Copy link
Contributor

Addressed issue where undetermined and not_applicable tests would not get flagged if a ruletest section had exclusively failures of those type. NOTE-- this will result in needing to revisit test cases that passed when they should have failed.

…et flagged if a ruletest section had exclusively failures of those type.
@jugonzal07 jugonzal07 added bug Something isn't working rule_test labels Jul 27, 2024
@JacksonJ-KC
Copy link
Collaborator

Hi @jugonzal07 I am going to get started addressing the test case failures. I will start another branch with a draft PR and we can merge that branch into this one once complete to hopefully pass the CI

@JacksonJ-KC
Copy link
Collaborator

JacksonJ-KC commented Jul 31, 2024

@jugonzal07 Take a look at the CI for the testing of rules 5-5, 5-7, 5-9, 5-11 in this PR. We are not getting any FAILURE messages, it just doesn't say "All tests passed!" at the conclusion of the rule's tests. Can we get these scenarios to raise a more obvious error?

image

…ndetermined_fix' into RT/JDJ/resolve-na-undetermined-failures

# Conflicts:
#	rct229/ruletest_engine/ruletest_engine.py
@jugonzal07
Copy link
Contributor Author

Very weird! I will definitely take a look. thanks @JacksonJ-KC

gonz102 and others added 2 commits July 31, 2024 16:40
…ctionary with an empty list of results, where no indication of outcome is given. This will now correctly log this error rather than failing, but producing no context on why it fails. Also, added additional check in case some other combination of factors results in a failure without a log describing why it fails.
…ndetermined_fix' into RT/JDJ/resolve-na-undetermined-failures
@jugonzal07
Copy link
Contributor Author

I found the issue and added a correction. In short, it looks like these tests are evaluating to nothing at all and return a blank list. This raises failures thankfully, but results in an empty log. My new commit will now correctly log this error. Also, added additional check in case some other combination of factors results in a failure without a log describing why it fails-- at least some log message will let us know.

@JacksonJ-KC
Copy link
Collaborator

Perfect, thanks!

This was referenced Aug 1, 2024
@JacksonJ-KC
Copy link
Collaborator

JacksonJ-KC commented Aug 1, 2024

Many rules in sections 5 and 22 use list filter to figure out if a rule is applicable. If list filter returns an empty list, then the rule is not evaluated at all and the NOT_APPLICABLE outcome is not actually being received.

I think (for rule tests) if context_list is empty then we should continue to provide the FAILURE message that you added, but if filtered_context_list is empty we should provide NOT_APPLICABLE outcome. If it's possible for this only to happen in rule tests then I think that would be preferable. I like that the rules simply don't evaluate when they don't apply to real projects.

Rules I have noticed that behave this way:
5-3
5-5
5-7
5-9
5-11
6-2
22-8 (if less than 300 tons)
22-9 (if less than 300 tons)
22-17
22-18
22-19
22-28

weilixu and others added 28 commits August 6, 2024 16:29
RS/JDJ/22-17, 22-18, 22-28 Applicability
…tem from being assigned to a multiple zones. This allows us to test cases when a user might accidentally do this. Also, potentially fixes a particular case where multizone systems did not always have the correct system assigned to their terminals (saw it in an edge case).
RT/JDJ/19-3 delete invalid test jsons temporarily
…would be shallow copies across the user, proposed, and baseline.
@weilixu weilixu marked this pull request as ready for review August 13, 2024 17:20
@weilixu weilixu merged commit b16f5ce into develop Aug 13, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule_test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants