openlmis-contract-tests

Clone Tools
  • last updated a few seconds ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
OLMIS-5668: Added endpoint to get user by id
OLMIS-5668: Added endpoint to get user by id
I see that most of the changes have been made aside from the first one: "Given I am logged in as a service-client". Is that one difficult or is it simply a WIP?

I see that most of the changes have been made aside from the first one: "Given I am logged in as a service-client". Is that one difficult or is it simply a WIP?

I think the more-right solution is the first one. The second one likely is okay. The last is not desirable at all.

I think the more-right solution is the first one. The second one likely is okay. The last is not desirable at all.

LocationTests was where the step was used. LocationTests.feature file is updated.

LocationTests was where the step was used. LocationTests.feature file is updated.

I was not sure about this. I have seen 3 patterns in the contract test around versions. Version number specified in the .env file. An explicit version specified inline. (Redis) No version specifie...

I was not sure about this. I have seen 3 patterns in the contract test around versions.

Version number specified in the .env file.
An explicit version specified inline. (Redis)
No version specified at all (FTP server)

https://github.com/OpenLMIS/openlmis-contract-tests/blob/c1a5b6789ab711f038f8e1c7b3a67a0b93501785/docker-compose.fulfillment.yml#L38

Is the version in the .env file for 'all' docker images that we use? I will update wiremock with version in the .env for now.

Looking good Elias, left a couple comments. Thanks.

Looking good Elias, left a couple comments. Thanks.

Ah, got it, thanks.

Ah, got it, thanks.

Yes, this token is important. It is coming from the subscription object. This header is used to authenticate at the upstream FHIR server. https://github.com/OpenLMIS/openlmis-contract-tests/blob/c...

Yes, this token is important. It is coming from the subscription object. This header is used to authenticate at the upstream FHIR server.

https://github.com/OpenLMIS/openlmis-contract-tests/blob/c1a5b6789ab711f038f8e1c7b3a67a0b93501785/src/cucumber/resources/org/openlmis/contract_tests/hapifhir/SubscriptionTests.feature#L18

I don't see changes in other features that used the old version of this step

I don't see changes in other features that used the old version of this step

Is this important? I don't see that bearer token in the steps so I'm confused where that one came from?

Is this important? I don't see that bearer token in the steps so I'm confused where that one came from?

Appears as if this Location UUID could be extracted (used 3 times)

Appears as if this Location UUID could be extracted (used 3 times)

Great to see wiremock in use here. Should the version here follow the format of the other services?

Great to see wiremock in use here.

Should the version here follow the format of the other services?

Great change to make this more BDD!

Great change to make this more BDD!

I understand this is outside the review, however I can see it so I'd recommend we make this more clear: "I am not logged in". This is minor.

I understand this is outside the review, however I can see it so I'd recommend we make this more clear: "I am not logged in".

This is minor.

The Given, When, Then isn't clearly identifiable, and we also see a bit too much technical detail here when we say "I stub a mock server". If possible I'd attempt to make this more focused on BDD'...

The Given, When, Then isn't clearly identifiable, and we also see a bit too much technical detail here when we say "I stub a mock server".

If possible I'd attempt to make this more focused on BDD's Given, When, Then: https://martinfowler.com/bliki/GivenWhenThen.html

Perhaps that'd read as:

  • Given I am logged in as a service client
  • And I have an upstream FHIR server
  • When my upstream FHIR server subscribes to Location updates with the OpenLMIS FHIR Service
    <JSON here>
  • And I update an OpenLMIS Location
  • Then after I pause for 30 seconds
  • And I verify that my Upstream FHIR Server has received a notification of a Location change
    <JSON here>


I think this is easier to read and will result in a more maintainable test over time.

Maybe we should increase the wait time to about 30 seconds. Mainly because I notice that tests on CI needs more time.

Maybe we should increase the wait time to about 30 seconds. Mainly because I notice that tests on CI needs more time.

I would say we could merge this step with one in the LoginStepDefs: ^I pause (\d+) seconds for right assignment regeneration$ by simply removing the part * for right assignment regeneration$*

I would say we could merge this step with one in the LoginStepDefs: ^I pause (\d+) seconds for right assignment regeneration$ by simply removing the part * for right assignment regeneration$*

OLMIS-5619: Add contract test for HAPI FHIR subscription.
OLMIS-5619: Add contract test for HAPI FHIR subscription.
done

done

I remember that we some talk about having a full structure of DTO because of caching/make sure that two objects are same.

I remember that we some talk about having a full structure of DTO because of caching/make sure that two objects are same.

from what I see we don't use this class so I will merge it with normal one

from what I see we don't use this class so I will merge it with normal one

I remember that we some talk about having a full structure of DTO because of caching/make sure that two objects are same.

I remember that we some talk about having a full structure of DTO because of caching/make sure that two objects are same.