Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

FI-569: Add chained search tests #411

Merged
merged 5 commits into from
Jan 17, 2020
Merged

Conversation

Jammjammjamm
Copy link
Contributor

@Jammjammjamm Jammjammjamm commented Jan 6, 2020

This branch adds a test for the chained search parameters US Core requires for PractitionerRole (practitioner.name and practitioner.identifier). Although this test was added to the US Core generator, the test itself and its unit test are hard coded. Implementing chained search tests in a generic manner would require a much larger amount of effort and isn't necessary to support these two chained parameters.

Submitter:

  • This pull request describes why these changes were made
  • Internal ticket for this PR: https://oncprojectracking.healthit.gov/support/browse/FI-569
  • Internal ticket links to this PR
  • Internal ticket is properly labeled (Community/Program)
  • Internal ticket has a justification for its Community/Program label
  • Code diff has been reviewed for extraneous/missing code
  • Tests are included and test edge cases
  • Tests/code quality metrics have been run locally and pass

Reviewer 1:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure
    where appropriate, and accomplishes the task's purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

Reviewer 2:

Name:

  • Code is maintainable and reusable, reuses existing code and infrastructure
    where appropriate, and accomplishes the task's purpose
  • The tests appropriately test the new code, including edge cases
  • You have tried to break the code

@Jammjammjamm Jammjammjamm self-assigned this Jan 6, 2020
@@ -306,6 +309,71 @@ def create_search_test(sequence, search_param)
)
end

def create_chained_search_test(sequence, search_param)
# NOTE: This test is currently hard-coded because chained searches are
Copy link
Contributor

Choose a reason for hiding this comment

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

could you raise an error in the generator if sequence not for PractionerRole just in case more are added in a future version?

@arscan
Copy link
Contributor

arscan commented Jan 15, 2020

Could you rebase when you have a chance? We can get this one in next.

@Jammjammjamm Jammjammjamm force-pushed the fi-569-add-chained-search-tests branch from cbb3ff8 to 39f8ba9 Compare January 16, 2020 15:13
arscan
arscan previously approved these changes Jan 16, 2020
@arscan arscan dismissed their stale review January 16, 2020 18:36

Test failure.

Copy link
Contributor

@arscan arscan left a comment

Choose a reason for hiding this comment

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

Noted the transitory build issue, which wasn't resolved. We'll address that later.

For future reference: https://travis-ci.org/onc-healthit/inferno/builds/633422948?utm_medium=notification&utm_source=github_status

Otherwise looks good!

@arscan arscan merged commit 5a43465 into development Jan 17, 2020
@arscan arscan deleted the fi-569-add-chained-search-tests branch January 17, 2020 16:58
@radamson radamson mentioned this pull request Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants