Skip to content

Add test-data-generator#326

Merged
nickevansuk merged 36 commits into
coverage/authenticationfrom
coverage/test-data-generator
Apr 5, 2021
Merged

Add test-data-generator#326
nickevansuk merged 36 commits into
coverage/authenticationfrom
coverage/test-data-generator

Conversation

@nickevansuk
Copy link
Copy Markdown
Collaborator

@nickevansuk nickevansuk commented Nov 16, 2020

A PR for generating test data reliably

Work remaining

Node.js

  • Update existing criteria
  • Add unit tests for criteria (test min and max bounds)
  • Have documentation generator produce JSON file for criteria needed per feature
  • Write test data generator from above
  • Amend controlled tests interface to use “hints/requirements”

Spec

  • Update test interface spec using hints/requirements

.NET

  • Implement hints/requirements for controlled data
  • Read test data file if exists from command line

Also this from my rough notes if helpful:

/* The documentation generator writes criteria requirements as JSON file */
{ 
  "feature1": {
    "Criteria1": 1,
    "Criteria2": 4,
  },
  "feature2": {
    "Criteria1": 1,
    "Criteria2": 4,
  }
},

Comment on lines +44 to +45
"test:startDateMin": "2020-12-20",
"test:startDateMax": "2020-12-25"
Copy link
Copy Markdown
Collaborator Author

@nickevansuk nickevansuk Nov 19, 2020

Choose a reason for hiding this comment

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

Just on this part of the model, I wonder whether we should have the requirements within a single type - given this is how they're likely processed, and also given that the requirements include a mix of opportunityRequirements and orderRequirements, so perhaps don't make sense on the Event itself, so e.g.

Suggested change
"test:startDateMin": "2020-12-20",
"test:startDateMax": "2020-12-25"
"test:testDataRequirements": {
"@type": "test:TestDataRequirements",
"test:startDateMin": "2020-12-20",
"test:startDateMax": "2020-12-25"
}

Alternatively perhaps we create a separation between opportunity and offer requirements...? This might make it more obvious which is which when processing, and also mirrors the separation that exists within the existing criteria?

Suggested change
"test:startDateMin": "2020-12-20",
"test:startDateMax": "2020-12-25"
"test:testOpportunityDataRequirements": {
"@type": "test:OpportunityTestDataRequirements",
"test:startDateMin": "2020-12-20",
"test:startDateMax": "2020-12-25"
}
"test:testOfferDataRequirements": {
"@type": "test:OfferTestDataRequirements",
"test:availableChannelIncludes": [
"availableChannelIncludes"
]
}

Copy link
Copy Markdown
Contributor

@lukehesluke lukehesluke Nov 19, 2020

Choose a reason for hiding this comment

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

@nickevansuk I agree with both suggestions.

Also, I wonder if we should structure the fields themselves? There's turning out to be a lot of different fields. It would be easier to implement (for external Booking Systems) if we abstracted the commonalities.

I've given a rough idea of what this might look like here at the top of TestDataRequirements.d.ts (and you can also see how many different fields there are so far. I've extended your suffix scheme for organising them)

If the fields are unstructured, a BS's logic might look like (for a controlled "create opportunity" request):

const prepayment = generateChoice(PREPAYMENT_OPTIONS, {
  allowlist: testDataRequirements.prepaymentAllowlist,
  blocklist: null,
  allowNull: testDataRequirements.prepaymentAllowNull,
});
const eventStatus = generateChoice(EVENT_STATUS_OPTIONS, {
  allowlist: null,
  blocklist: testDataRequirements.eventStatusBlocklist,
  allowNull: null
});

EW. And what if we eventually add a prepaymentBlocklist field - they'd have to add it (or more likely - not realise and start generating incorrect data).

If the fields are structured, the same BS's logic might look like:

const prepayment = generateField(testDataRequirements.prepayment);
const eventStatus = generateField(testDataRequirements.eventStatus);

The field could look like:

prepayment: {
  type: 'Allowlist',
  mustBeOneOf: ['https://openactive.io/Unavailable'],
  valueType: 'oa:RequiredStatusType',
}

Therefore, the BS's generateField(..) function could generate everything it needs to from that definition, and would allow for adding new criteria or updating existing criteria without breaking existing BSs (unless we add a new type of requirement OR add new fields)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Love it!

lukehesluke and others added 8 commits November 19, 2020 14:23
…FiveSpaces, -FlowRequirementOnlyApproval, -Free
* update merging logic so it just errors if there are overlapping requirements (this can be fixed as and when) ; TS all passes now
…s/criteria-requirements.json

Also, cleanup documentation generator a bit and use __dirname for file paths so that script wont fail if run from a different directory than originally designed
@lukehesluke
Copy link
Copy Markdown
Contributor

@nickevansuk

From your Node.js checklist above, what new criteria is this point referring to?

Add new criteria

* refactor: Move createTestInterfaceOpportunity(..) out of RequestHelper so it can also be used by test-data-generator script
* chore: Type TestInterfaceOpportunity
@nickevansuk nickevansuk force-pushed the coverage/test-data-generator branch from f3c8404 to 73c27ee Compare April 1, 2021 15:41
@nickevansuk nickevansuk merged commit f1b828c into coverage/authentication Apr 5, 2021
@nickevansuk nickevansuk deleted the coverage/test-data-generator branch April 5, 2021 22:16
@nickevansuk
Copy link
Copy Markdown
Collaborator Author

Note merging this Node.js work ahead of completion of other aspects to consolidate branches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants