Closed elipe17 closed 1 week ago
Attention: Patch coverage is 92.75362%
with 30 lines
in your changes missing coverage. Please review.
Project coverage is 92.97%. Comparing base (
14ca954
) to head (c503e1d
). Report is 1 commits behind head on develop.
I did a test with ssp_section1_datafile.txt
file, and added another duplicate trying to mimic a user adding a line three times by mistake and the dup errors were handled correctly.
One last gut check @elipe17 - 36k lines were removed from tdrs-backend/tdpservice/parsers/test/data/ADS.E2J.NDM1.TS53_fake.rollback.txt
, was that intentional?
One last gut check @elipe17 - 36k lines were removed from
tdrs-backend/tdpservice/parsers/test/data/ADS.E2J.NDM1.TS53_fake.rollback.txt
, was that intentional?
@jtimpe, that is an intentional deletion. Since I changed the logic for rollback/record deletion it became very import for the test test_parse_big_s1_file_with_rollback
to start running. But it was unnecessary to have as many lines in the file as we did to test the logic. So I removed all lines that were unnecessary to test the cases covered in that test.
@elipe17 below are my notes so far from qasp review. I've included test files as well for convenience.
Partial duplicate record detected with record type T1 at line 5. Record is a partial duplicate of the record at line number 2. Duplicated fields causing error: record type, reporting month and year, and case number.
:grey_question:test files:
section1 2795_partdups_S1.txt 2795_exactdups_S1.txt
section2 2795_partdups_s2.txt 2795_exactdups_s2.txt
section3 2795_dups_s3.txt section4 2795_dups_s4.txt
Summary of Changes
CPU and Memory Analysis
web
container stats during parsing of the largest file we have in the repo (~50MB) on thedevelop
branch and this branch.super_big_file_results.txt
. The data listed in that text file can be found in the accompanying files:develop_super_big_file.txt
anddup_super_big_file.txt
.track_docker_stats.sh.txt
(GitHub won't let you upload a pure shell file). super_big_file_results.txt dup_super_big_file.txt develop_super_big_file.txt track_docker_stats.sh.txtHow to Test
List the steps to test the PR These steps are generic, please adjust as necessary.
Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
Deliverable 2: Tested Code
CodeCov Report
comment in PR)CodeCov Report
comment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjolly
andttran-hub
using Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure