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

Consolidate front-end types/fixtures and back-end data #3027

Closed
philippotto opened this issue Aug 9, 2018 · 4 comments · Fixed by #3065
Closed

Consolidate front-end types/fixtures and back-end data #3027

philippotto opened this issue Aug 9, 2018 · 4 comments · Fixed by #3065
Assignees
Milestone

Comments

@philippotto
Copy link
Member

No description provided.

@daniel-wer
Copy link
Member

I've evaluated two different approaches for this:

(1) Use the existing snapshots of the data returned by the admin_rest_api requests (from the e2e tests) to check concordance with the frontend flow types.
In theory this is feasible, however, it is rather hacky and somewhat tricky. We would need to:

  • Read the snapshot files and rewrite them, so that each individual snapshot object is prefixed with const object: SpecificFlowType = . So we would need to know the SpecificFlowType of each snapshot object and we would need to correctly import said flow type (most of them should be in api_flow_types.
  • Some volatile fields are replaced with static values. The replaced value always is a string for now, which changes the type of that property sometimes. We would need to replace volatile values with a static value of the correct type.

(2) Use the flow-runtime library to check flow types during runtime (during test runs).
In theory, this is a rather elegant solution as we could add specific type assertions to our e2e tests, where we retrieve the backend data anyways. However, flow-runtime currently crashes for our code base, mostly due to bugs related to object spreads. (gajus/flow-runtime#203, gajus/flow-runtime#204)
Fixing these issues is not easily possible (I tried for ~2 hours, but didn't have success).

@philippotto
Copy link
Member Author

Thanks for evaluating this! Maybe we should postpone the issue until (2) becomes feasible?

  • Read the snapshot files and rewrite them, so that each individual snapshot object is prefixed with const object: SpecificFlowType =. So we would need to know the SpecificFlowType of each snapshot object and we would need to correctly import said flow type (most of them should be in api_flow_types.
  • Some volatile fields are replaced with static values. The replaced value always is a string for now, which changes the type of that property sometimes. We would need to replace volatile values with a static value of the correct type.

Alternatively, we could make it more explicit and independent of the snapshots by writing code like:

  const createdExplorational = await api.createExplorational(dataSetName, "skeleton", false);
  writeFlowCheckingFile(createdExplorational, "APIAnnotationType");

  t.snapshot(replaceVolatileValues(createdExplorational), {
    id: "annotations-createExplorational",
  });

Then, we wouldn't need to fight with volatile values or with rewriting snapshots.
writeFlowCheckingFile would create a new file for each type in a new directory for which we could then run flow.

... we would need to know the SpecificFlowType of each snapshot object and we would need to correctly import said flow type (most of them should be in api_flow_types).

Yeah, for testing the back-end routes that should be enough. Might be a good chance to ensure that all backend types are contained in this file.

Overall, I'm not too sure about the benefit/cost ratio, though. Maybe we should discuss this in person tomorrow :)

@daniel-wer
Copy link
Member

Sounds good, let's discuss this in person and I'll update this issue afterwards :)

@daniel-wer
Copy link
Member

We agreed on trying flow-runtime again at a later time, maybe 6 months from now and check whether it works better then. In the meantime we'll prototype the first suggestion using @philippotto's writeFlowCheckingFile approach and see how well it works.

@philippotto philippotto added this to the Sprint 26a milestone Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants