-
Notifications
You must be signed in to change notification settings - Fork 9
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
docs: Write a guide for creating a new test #565
Conversation
Co-authored-by: civsiv <civ@imin.co>
And, a guide for Opportunity Criteria
@@ -0,0 +1,3 @@ | |||
# Request Templates | |||
|
|||
TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue to create this: #566
@@ -0,0 +1,273 @@ | |||
# Contributing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved DEVELOPMENT
to CONTRIBUTING
as I believe this is a more common place to find this kind of information. The information in here has ballooned a little and I wonder if there are some better places for some of it. But with this PR it's at least written down somewhere!
|
||
### 2. New Test Interface Criteria? | ||
|
||
Each test has a set of Criteria configured (see [Opportunity Criteria](#opportunity-criteria)). Check whether or not this test can use a Criteria that already exists or if a new one is needed. If a new Criteria is needed, be sure to create it in the [Test Interface](https://openactive.io/test-interface/) spec, as well as in the [`test-interface-criteria`](../test-interface-criteria/) package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't gone into more detail here about how to create a new one. For the Test Suite part, there is an issue for that here: #554. Not sure if we need documentation for the Test Interface side
|
||
### Opportunity Criteria | ||
|
||
What a Criteria is is documented in [the test-interface-criteria docs](../test-interface-criteria/README.md#criteria). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads slightly funny to me, maybe something like:
What a Criteria is is documented in [the test-interface-criteria docs](../test-interface-criteria/README.md#criteria). | |
Each Test Interface Criteria is defined and documented in [the test-interface-criteria docs](../test-interface-criteria/README.md#criteria). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I've done this slightly differently as it is not the case that each criteria is defined in that doc
|
||
If you need to test a different flow from this, there are many other Flow Stage Recipes available, and it's always possible to create custom flows from scratch. | ||
|
||
Note that the Flow Stages are just initiated. None of the API calls are made yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the Flow Stages are just initiated. None of the API calls are made yet. | |
Note that the Flow Stages are just initiated during set up. None of the API calls are made yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Done
@@ -21,6 +21,44 @@ const TestOpportunityBookableCriteria = TestInterfaceCriteria.criteriaMap.get('T | |||
const { matchesCriteria, unmetCriteriaDetails } = TestInterfaceCriteria.testMatch(TestOpportunityBookableCriteria, opportunity); | |||
``` | |||
|
|||
## Concepts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on this section being above Usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My rationale was that it seems to me to be a common pattern for usage/examples to come first in project READMEs. Perhaps geared towards people who are already somewhat knowledgeable and want to get stuck in
Co-authored-by: civsiv <civ@imin.co>
And, a guide for Opportunity Criteria
Closes #561 and #564