data-to-insight / csc-validator-be-cin

1 stars 1 forks source link

Fix/stringify codes #459

Closed tab1tha closed 10 months ago

tab1tha commented 10 months ago

The previous change made on this branch, to change the rule codes to strings at the point where they are read into the registry, caused the in-rule-file tests to fail. This was because, the type conversion happened in the validation func so that by the time the test_validation function was being run, the rule code had changed already. For example, for a rule defined with code=100 (int) the @rule_definition decorator will read it in, convert, then store it as a string. such that by the time we get to the test_validate function, assert rule_code == 100 will fail. instead we'd have to do assert rule_code= "100"

Considerations:

The changes in this pull request implement the second consideration and updates the type expectation in the definition of rule_definition such that it expects only str rule codes and will flag if otherwise.

codecov-commenter commented 10 months ago

Codecov Report

Merging #459 (5e9285c) into main (f9495f4) will decrease coverage by 0.09%. Report is 1 commits behind head on main. The diff coverage is 94.56%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #459      +/-   ##
==========================================
- Coverage   93.13%   93.05%   -0.09%     
==========================================
  Files         123      123              
  Lines        5083     5096      +13     
==========================================
+ Hits         4734     4742       +8     
- Misses        349      354       +5     
Files Changed Coverage Δ
cin_validator/cin_validator.py 12.85% <0.00%> (-0.10%) :arrow_down:
cin_validator/rule_engine/__registry.py 100.00% <ø> (ø)
cin_validator/rules/cin2022_23/rule_2883.py 100.00% <ø> (ø)
cin_validator/__main__.py 53.48% <69.23%> (+2.13%) :arrow_up:
cin_validator/rules/cin2022_23/rule_100.py 100.00% <100.00%> (ø)
cin_validator/rules/cin2022_23/rule_1103.py 100.00% <100.00%> (ø)
cin_validator/rules/cin2022_23/rule_1104.py 100.00% <100.00%> (ø)
cin_validator/rules/cin2022_23/rule_1105.py 100.00% <100.00%> (ø)
cin_validator/rules/cin2022_23/rule_1510.py 100.00% <100.00%> (ø)
cin_validator/rules/cin2022_23/rule_1520.py 100.00% <100.00%> (ø)
... and 72 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

tab1tha commented 10 months ago

Again, how were these failing tests not caught when the earlier pull request was made? Somehow they managed to pass Github automated testing.

SLornieCYC commented 10 months ago

Again, how were these failing tests not caught when the earlier pull request was made? Somehow they managed to pass Github automated testing.

Are the github actions configured to checkout the relevant branch when they run? I think actions/checkout uses the repository main branch unless told otherwise.

tab1tha commented 10 months ago

Thank you for digging into this @SLornieCYC The documentation you linked to is helpful. I've gone through, and it seems that if we implement it to point to the branch we're on, we'll have to edit the yaml file each time we create a new branch. Initially, it seemed that writing my-branch as is done in the tutorial will guess what branch you are on but when I searched the repo, I couldn't find any reference to a my-branch variable apart from the one stated in the documentation. So it is a placeholder that has to be manually edited each time.

However, if we leave it to run on on the main branch, as it does now, then it checks if the incoming changes might cause the main branch to fail. This works sometimes (since some pull requests have failed tests while being on non-main branches) but seems to not work all the time.

SLornieCYC commented 10 months ago

Can't you use the github environment variable there in place of my-branch?

E.g. something like: (Not 100% sure on the right syntax)

uses: actions/checkout@v4
with:
ref: ${{ github.head_ref || github.ref_name }}

https://stackoverflow.com/questions/60300169/how-to-get-branch-name-on-github-action https://stackoverflow.com/a/71158878