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.

Sign up for GitHub

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

Jump to bottom

Add Golden Copy Tests #1674

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open

Add Golden Copy Tests #1674

wants to merge 54 commits into from

Conversation

pluckyswan
Copy link
Contributor

Description

Under construction...

Issue

#1600

Checklist

  • I have added tests to cover my changes
  • I have added logging where useful (with appropriate log level)
  • I have added JavaDocs where required
  • I have updated the documentation accordingly

Note: You may remove items that are not applicable

jherrflexion and others added 30 commits December 16, 2024 16:56
@jherrflexion
WIP Initial setup
07b96d1
@pluckyswan
add notes for later and initial setup/cleanup
2d14e9a
@jherrflexion @somesylvie @saquino0827
Added basic compare
f8afd54 
Co-Authored-By: Sylvie <38440028+somesylvie@users.noreply.github.com>
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
@jherrflexion @saquino0827 @pluckyswan
Testing workflow dispatch
dad2cd3 
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
Co-Authored-By: Bella L. Quintero <96704946+pluckyswan@users.noreply.github.com>
@jherrflexion @saquino0827 @pluckyswan
Aded Golden tests to gradle
1d8877c 
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
Co-Authored-By: Bella L. Quintero <96704946+pluckyswan@users.noreply.github.com>
@jherrflexion @saquino0827 @pluckyswan
Adding Golden tests to Workflows
9b81688 
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
Co-Authored-By: Bella L. Quintero <96704946+pluckyswan@users.noreply.github.com>
@jherrflexion @saquino0827 @pluckyswan
Added fetch parameter
d4867f7 
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
Co-Authored-By: Bella L. Quintero <96704946+pluckyswan@users.noreply.github.com>
@jherrflexion @saquino0827 @pluckyswan
Removed comments
87dbc14 
Co-Authored-By: Samuel Aquino <saquino@flexion.us>
Co-Authored-By: Bella L. Quintero <96704946+pluckyswan@users.noreply.github.com>
@pluckyswan @saquino0827
add input files and delete unused input files
5bef9f0 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
refactor file fetcher and implementation
83edb87 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan
refactor file fetcher calls
084387d
@pluckyswan @saquino0827
add env to workflow and rename file path
1e49731 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan
modify example files to be aligned with readme reqs
3de1187
@pluckyswan
move directory up one and fix file paths
ebe7a46
@pluckyswan
move golden copy to automated to get files after transformed
9605cc1
@pluckyswan @saquino0827
use correct syntax
f63c56c 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
make variables strings
de61fb5 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
add dollar sign to echo
96cae6c 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
try fixing format
e17f1e9 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
change syntax
0747ee8 
Co-authored-by: saquino0827 <saquino@flexion.us>
@pluckyswan @saquino0827
@somesylvie @jcrichlake
remove line break at end of line in example files
bdbed53 
Co-authored-by: saquino0827 <saquino@flexion.us>
Co-authored-by: Sylvie <sschuresko@flexion.us>
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jcrichlake
add double quotes to if statement
71a243d 
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jcrichlake
add golden copy versions of example files and revert to og directory …
24e5ac3 
…structure

Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jcrichlake
modify msh-10 header in example file
401c50f 
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jcrichlake
fix syntax
229af63 
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jcrichlake @jherrflexion
remove breaks from example file
9485cda 
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion @jcrichlake
add example file
3c850de 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: jcrichlake <145698165+jcrichlake@users.noreply.github.com>
@pluckyswan @jherrflexion
remove comment, add quotes to variable, and create workflow file
1f0dde2 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
remove done
d7623ae 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
remove unused workflow
92c039c 
Co-authored-by: James Herr <jherr@flexion.us>
pluckyswan and others added 11 commits December 19, 2024 16:14
@pluckyswan
separate workflows and update readme
5b648b7
@pluckyswan
add notes on next steps
7b545da
@pluckyswan @jherrflexion @somesylvie
stub out filter for organizing and fetching azure files
ccca6ab 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: Sylvie <sschuresko@flexion.us>
@pluckyswan @jherrflexion
modify filter for file paths
6873446 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
update readme and file examples base on filter for new receiver
9fbe6fa 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
fix uses pathing
885cd21 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
fix uses pathing
bc83820 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
add workflow dispatch
cf0489f 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
combine submit workflows into one and update readmes
91371ae 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
delete unused workflow
000d590 
Co-authored-by: James Herr <jherr@flexion.us>
@pluckyswan @jherrflexion
rename reusable workflow
06558f7 
Co-authored-by: James Herr <jherr@flexion.us>
@github-actions GitHub Actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

1600 - Partially compliant

Fully compliant requirements:

  • Modify automated staging test workflows (submit and run).
  • Create test.
  • Add input and expected files.
  • Update/add README.

Not compliant requirements:

  • Every validation message from the UCSD implementation is tested as defined in the provided spreadsheet.
  • Golden copy tests failing do not impact whether or not existing RS e2e test will run, and vice versa.
  • Possible modify cron schedule if needed for golden copy.
  • Add new receivers in RS.
  • Test.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Environment Variable Dependency

The code heavily relies on the 'LOCAL_FILE_PATH' environment variable without a fallback, which could lead to runtime errors if not set.

        String files_path = System.getenv("LOCAL_FILE_PATH");
        if (files_path == null || files_path.isEmpty()) {
            throw new IllegalArgumentException("Environment variable LOCAL_FILE_PATH is not set");
        }

github-actions[bot]
github-actions bot reviewed Dec 23, 2024
View reviewed changes
rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java
// when its pulled down and modify destinationName to be test folder specific
// possibly use a different receiver and filter on that

String testTypeAndSourceName = "Automated/" + sourceName;
Copy link

Choose a reason for hiding this comment

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

Consider refactoring the blob naming and organizing logic into a separate method or class to improve modularity and testability. This change would help isolate the blob management logic, making the code cleaner and easier to maintain. [important]

rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/GoldenCopyTest.groovy
for (filePair in matchedFiles) {
def actualFile = filePair.getKey()
def expectedFile = filePair.getValue()
if (!actualFile.equals(expectedFile)) {
Copy link

Choose a reason for hiding this comment

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

Implement additional logging for each file comparison in the 'Compare files' test to provide more detailed feedback on which files are being compared and the result of each comparison. This enhancement would be beneficial for debugging and understanding test failures. [medium]

rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java
if (files_path.contains("GoldenCopy")) {
pathPrefix += datePrefix + "GoldenCopy/";
}

ListBlobsOptions options = new ListBlobsOptions().setPrefix(pathPrefix);
Copy link

Choose a reason for hiding this comment

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

Add error handling for the Azure blob fetching process to manage exceptions and provide meaningful error messages if the fetching fails. This would improve the robustness of the file fetching mechanism. [important]

@github-actions GitHub Actions
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

pluckyswan and others added 11 commits December 23, 2024 14:30
@pluckyswan @jherrflexion @basiliskus
readd the og workflow and delete new one
0fba3fb 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan @jherrflexion @basiliskus
move folders around
a9cf564 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan @jherrflexion @basiliskus
fix file changes
7a13855 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan @jherrflexion @basiliskus
change file path
8ca3337 
Co-authored-by: James Herr <jherr@flexion.us>
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan @basiliskus
remove semi colon
5086197 
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan @basiliskus
mess with pathing regex
85b11b6 
Co-authored-by: basiliskus <541149+basiliskus@users.noreply.github.com>
@pluckyswan
correct naming in workflow
17a5382
@pluckyswan
use correct inputs syntax
77d9c71
@pluckyswan
workflow fixes
b67cd88
@pluckyswan
pass secrets to workflow
7143562
@pluckyswan
fix file path and exclude from test
c87d402
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pluckyswan @jherrflexion