fhir-data-pipes icon indicating copy to clipboard operation
fhir-data-pipes copied to clipboard

Migrate FhirStoreUtilTest to Junit5

Open meetmehta1198 opened this issue 1 year ago • 4 comments

Description of what I changed

Fixes #1103

E2E test

TESTED:

Please replace this with a description of how you tested your PR beyond the automated e2e/unit tests.

Checklist: I completed these to help reviewers :)

  • [x] I have read and will follow the review process.

  • [x] I am familiar with Google Style Guides for the language I have coded in.

    No? Please take some time and review Java and Python style guides.

  • [x] My IDE is configured to follow the Google code styles.

    No? Unsure? -> configure your IDE.

  • [x] I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)

  • [x] I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.

  • [x] All new and existing tests passed.

  • [x] My pull request is based on the latest changes of the master branch.

    No? Unsure? -> execute command git pull --rebase upstream master

meetmehta1198 avatar Aug 01 '24 01:08 meetmehta1198

This PR solve some of the test cases based on the Issue #1103

@chandrashekar-s

meetmehta1198 avatar Aug 06 '24 01:08 meetmehta1198

cc @bashir2

meetmehta1198 avatar Aug 12 '24 23:08 meetmehta1198

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 51.67%. Comparing base (b03c2c7) to head (b209ce2).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1135      +/-   ##
============================================
- Coverage     51.76%   51.67%   -0.09%     
+ Complexity      669      668       -1     
============================================
  Files            95       95              
  Lines          5612     5612              
  Branches        731      731              
============================================
- Hits           2905     2900       -5     
- Misses         2425     2430       +5     
  Partials        282      282              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Aug 14 '24 18:08 codecov-commenter

@chandrashekar-s can you please review this PR

meetmehta1198 avatar Sep 17 '24 17:09 meetmehta1198

Hi @meetmehta1198 for the changes, there is an build error which you can see from the logs. Can you please have a look at it.

chandrashekar-s avatar Nov 14 '24 11:11 chandrashekar-s

Hi @meetmehta1198, just a reminder to look into the build issue. Please let me know if you face any issues while accessing the build logs.

chandrashekar-s avatar Nov 22 '24 07:11 chandrashekar-s

I will take a look into it! Thanks

meetmehta1198 avatar Nov 26 '24 00:11 meetmehta1198

@chandrashekar-s I have updated the code and the build is successful. Can you please approve this PR?

meetmehta1198 avatar Nov 26 '24 02:11 meetmehta1198

Thanks @chandrashekar-s for all the feedback and approval. Great experience working with you!

meetmehta1198 avatar Nov 29 '24 04:11 meetmehta1198

Only those with write access to this repository can merge pull requests.

I do not have necessary permissions to merge code!

meetmehta1198 avatar Nov 29 '24 04:11 meetmehta1198

Thanks @meetmehta1198 for this and congratulations on your first contribution to this repo. Just one minor comment: It seems to me that from #1103 you are fixing tests under the common module only, correct? Then I guess we should change the PR description as it currently says it fixes that bug (and will close that bug upon merge).

bashir2 avatar Nov 29 '24 06:11 bashir2

Thanks @meetmehta1198 for this and congratulations on your first contribution to this repo. Just one minor comment: It seems to me that from #1103 you are fixing tests under the common module only, correct? Then I guess we should change the PR description as it currently says it fixes that bug (and will close that bug upon merge).

I have updated the PR description, so that it does not close the main issue.

chandrashekar-s avatar Nov 29 '24 06:11 chandrashekar-s