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

fix: only restrict set of checks when executing them [DHIS2-16256] #15840

Merged
merged 6 commits into from
Dec 6, 2023

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Dec 6, 2023

Summary

When executing data integrity checks the default (no set of checks was explicitly specified) is to only run checks that are not marked as "slow". This filter wrongly was also used when expanding checks to read the results from the cache. This meant that slow checks would not be included unless explicitly named.

Automatic Testing

Modified existing tests with additional checks that verify that reading check descriptions and check summaries after execution does include the slow checks which means they are not filtered.

Manual Testing

  • run checks POST /api/dataIntegrity/summary (no checks parameter to run all defaults)
  • run a specific slow check POST /api/dataIntegrity/summary?checks=data_elements_aggregate_abandoned
  • wait some
  • fetch results GET /api/dataIntegrity/summary should include the slow check data_elements_aggregate_abandoned

@jbee jbee self-assigned this Dec 6, 2023
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Merging #15840 (10a7db5) into master (895775c) will increase coverage by 0.00%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master   #15840   +/-   ##
=========================================
  Coverage     66.36%   66.37%           
- Complexity    31428    31435    +7     
=========================================
  Files          3487     3487           
  Lines        130028   130029    +1     
  Branches      15183    15183           
=========================================
+ Hits          86294    86302    +8     
+ Misses        36648    36645    -3     
+ Partials       7086     7082    -4     
Flag Coverage Δ
integration 50.05% <100.00%> (+<0.01%) ⬆️
integration-h2 32.34% <0.00%> (-0.01%) ⬇️
unit 30.42% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...his/dataintegrity/DefaultDataIntegrityService.java 78.15% <100.00%> (+1.46%) ⬆️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 895775c...10a7db5. Read the comment docs.

@jbee jbee enabled auto-merge (squash) December 6, 2023 09:37
Copy link
Contributor

@jason-p-pickering jason-p-pickering left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for fixing

Copy link

sonarqubecloud bot commented Dec 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@jbee jbee merged commit 64e4d62 into dhis2:master Dec 6, 2023
18 checks passed
jbee added a commit to jbee/dhis2-core that referenced this pull request Dec 6, 2023
…his2#15840)

* fix: only restrict set of checks when executing them [DHIS2-16256]

* test: show that slow tests are included in summary [DHIS2-16256]
jbee added a commit to jbee/dhis2-core that referenced this pull request Dec 6, 2023
…his2#15840)

* fix: only restrict set of checks when executing them [DHIS2-16256]

* test: show that slow tests are included in summary [DHIS2-16256]
jbee added a commit that referenced this pull request Dec 6, 2023
…15840) (#15848)

* fix: only restrict set of checks when executing them [DHIS2-16256]

* test: show that slow tests are included in summary [DHIS2-16256]
jbee added a commit that referenced this pull request Dec 6, 2023
…15840) (#15849)

* fix: only restrict set of checks when executing them [DHIS2-16256]

* test: show that slow tests are included in summary [DHIS2-16256]
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.

3 participants