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.

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

feat(simple-book-free-opportunities): Create with-erroneous-payment-property-test #127

Conversation

lukehesluke
Copy link
Contributor

Additionally:

  • Update breq so that it can use different templates if needed (e.g. to ensure that a B request includes a payment property for free opportunities)
  • Update without-payment-property-test so that it is always explicitly withholding the payment property
  • Partially type chakram, which does not, unfortunately have any community TS defines

…roperty-test

Additionally:

- Update breq so that it can use different templates if needed (e.g. to ensure that a B request includes a `payment` property for free opportunities)
- Update without-payment-property-test so that it is always explicitly withholding the `payment` property
- Partially type chakram, which does not, unfortunately have any community TS defines
@@ -76,16 +76,11 @@ function (configuration, orderItemCriteria, featureIsImplemented, logger, state,

return chakram.wait();
});

afterAll(async function () {
await state.deleteOrder();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on our earlier conversation, I've removed this from the default advice for new tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

@lukehesluke think I mentioned before, but I wonder if we should in fact make this configurable? As it might be that systems want to minimise the number of new orders created, especially in Random mode?

Copy link
Contributor Author

@lukehesluke lukehesluke Aug 6, 2020

Choose a reason for hiding this comment

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

That makes sense, @nickevansuk . So I'll leave this for now (as we'd probably want this configuration option to be exercised in the feature-helper rather than in the tests' afterAll blocks?) and then create an issue for setting this up. Sound good?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue: #142

Comment on lines +18 to +19
// This must also be TestOpportunityBookableFree as the payment property is only disallowed if ALL items are free.
controlOpportunityCriteria: 'TestOpportunityBookableFree',
Copy link
Contributor Author

@lukehesluke lukehesluke Aug 4, 2020

Choose a reason for hiding this comment

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

I just want to confirm that this makes sense. My theory so far is that the "control" is so that the "multiple order items" test has some normal opportunities as well as the specialised opportunities relevant to the test.

In this case, I've set the control to free because the use case can only be tested if the entire basket is free (payment is only forbidden if all opps are free)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep that's right, and makes sense

@lukehesluke lukehesluke marked this pull request as ready for review August 5, 2020 17:10
Copy link
Collaborator

@nickevansuk nickevansuk left a comment

Choose a reason for hiding this comment

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

Love the templating approach!

@lukehesluke lukehesluke merged commit 76cddaf into master Aug 14, 2020
@lukehesluke lukehesluke deleted the feature/simple-book-free-opportunities--with-erroneous-payment-property branch August 14, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants