FI-3518/FI-3519: Add basic Bulk Data client test suite, groups, and tests

[holmesie]

Summary

Add bulk data client test suite containing tests that verify the basic "happy path" bulk data export flow, and a test that verifies the a client's ability to delete a bulk data export request. Tests work by utilizing the wait functionality to capture all requests during the export flow, and then verify that the correct requests were made.

Testing Guidance

Includes postman collections for both test groups (written for the System type export). https://github.com/smart-on-fhir/bulk-data-client/tree/main is also a useful reference implementation to test against.

[emichaud998]

Including the Tags module is not necessary here. I believe if you add require_relative 'tags' to the client suite file, you can use the tag constants in all of the endpoint files.

[holmesie]

I'm not 100% sure what you mean. I'm bringing in the Tags module as a mixin to get access to the constants directly.

[emichaud998]

Oops nevermind with the way the Tags and URLs files are set up, you are right! I was looking at other test kits where it is done differently where the tags file does not have a Tags module defined. So for example for this test kit, the tags file would look like this:

module BulkDataTestKit
  module BulkDataV200
    module Client
      METADATA_TAG = 'bulk_data_metadata'
      PATIENT_KICKOFF_TAG = 'bulk_data_kickoff_patient'
      GROUP_KICKOFF_TAG = 'bulk_data_kickoff_group'
      SYSTEM_KICKOFF_TAG = 'bulk_data_kickoff_system'
      STATUS_TAG = 'bulk_data_status'
      OUTPUT_TAG = 'bulk_data_output'
      DELETE_TAG = 'bulk_data_delete'
    end
  end
end

Again maybe not a necessary change, but it does allow you to use constants from the Tags and URLs files without needing to include a new module.

[holmesie]

Oh I see what you mean - it's definitely better to be consistent with the other test kits. I've made this change.

[emichaud998]

Including ExportTypes I believe also is not necessary, since you are just using constants from that file like like the tags file

[holmesie]

Same comment as above (about Tags)

[emichaud998]

Same comment as above (about Tags) and how if a new ExportTypes module is not defined in the export types file, then you do not need to include the module and can instead use the constants directly in every file by requiring the export types file from the parent suite.

[holmesie]

Fixed.

[emichaud998]

Why does the suite_id need to be different for suite_endpoint? The suite id is just used to create the base url, so I think it should be able to just be self.class.suite.id

[holmesie]

The URLs module is also used in the endpoints (not just suite/tests/groups), which doesn't have suite on its class - so this extra case makes it work there by looking at the result that is available via the test id lookup. I might have missed something in the way I'm using the endpoints though that would make this work? I wasn't able to see an easy way when I was looking at it.

[emichaud998]

Okay yeah I see where you are using the suite_id in the url in the endpoints, so that makes sense. Not sure if there is a better way to make this work, I haven't seen this come up in other test kits. This solution definitely works and looks good to me but if there is a better way of addressing this issue, I am not entirely sure what that would be.

[emichaud998]

Not sure if this is something that should be changed, but I am thinking to be consistent with the rest of the bulk data test suites, there is no need to add the Client module here

[holmesie]

I think we want to separate out the client tests in some way, right? Do you mean move Client up one level e.g. have a module BulkDataV200Client?

[emichaud998]

Yes sorry that is what I meant, so rather than have a client module under the BulkDataV200 module, combine them so it is something like module BulkDataV200Client. Again not sure if this is a necessary change, I was just looking at the rest of the Bulk Data test kit and saw that the server tests do not have a server module.

[holmesie]

Yeah that makes sense, I've made this change.

[emichaud998]

Changes look great! Just left a few additional minor comments!

[emichaud998]

Looks great to me!

[karlnaden]

I'm not able to access these at these links, even correcting for the (temporary) branch. I think you need to replace v2.0.0/client with v2.0.0_client in the URLs.

[holmesie]

Fixed

[emichaud998]

One tiny change suggestion. I would maybe change the example token to SAMPLE_TOKEN both here and in the Postman collection as the starting default, since that is the access token most Inferno test kits use as the example test token.

[emichaud998]

Access token changes look good and work as expected! I left one tiny comment just for consistency with other test kits, but otherwise this looks good to me!