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

wip: feat/t0116 gateway cache.sh #30

Closed
wants to merge 18 commits into from
Closed

Conversation

galargh
Copy link
Contributor

@galargh galargh commented Apr 3, 2023

This PR ends up doing a bit more than I intended at first. It:

  • rewrites t0116 in sugar mode - 1cf1729
  • makes SugarTests the default for defining tests - 5dde175
  • makes SugarTests the only option for defining tests - 72453bc and cdddcc9
  • adds Provider interface (think `StringProvider(value).Get() rather than using value directly) - 284e1e6
  • adds a way to init header value from a variable - 8e967b3
  • adds a way to init header value with a function - f5cace9
  • enables t0116 test cases that depend on responses from other test cases - 4e7e048
  • adds comments on next steps - 2f9adaf

I mentioned some of the interesting commits up here. I think looking at it that way is much more convenient. We can definitely discuss this in person too!

Feedback is more than welcome.

I think embracing SugarTests would let us simplify quite a bit of the internals.

The way I implemented information sharing between requests... I'm not 100% convinced.

  • It works cause' test cases are executed sequentially, which is always true for sub-test cases.
  • If we were to exclude the dependency test case or run only the test that has dependencies, it'd fail. It might be fine because those are power user options anyway.
  • If the dependency test case failed, the dependee would fail too, but the error wouldn't indicate the reason too well. Maybe it's enough to check the request before making it and add some hints around it.

Other things for consideration:

  • We could group tests with dependencies more formally so that if the dependency fails, we don't even execute the latter test cases.
  • The request from the dependency could be done outside test cases. The downside is, any failure would hold back all the test cases.

Finally, important to note that this PR doesn't enable IPNS test cases.

@galargh galargh force-pushed the feat/t0116-gateway-cache.sh branch from 054e520 to 2f9adaf Compare April 4, 2023 09:55
@laurentsenta laurentsenta mentioned this pull request Apr 4, 2023
20 tasks
@laurentsenta laurentsenta removed their request for review June 8, 2023 08:30
@laurentsenta
Copy link
Contributor

(removing myself while this is still in draft)

@galargh
Copy link
Contributor Author

galargh commented Jun 12, 2023

For what it's worth, it's never getting out of draft but I'm keeping it alive to remember to salvage parts of it that are still relevant.

@laurentsenta laurentsenta mentioned this pull request Jun 28, 2023
@galargh galargh closed this Jun 28, 2023
@galargh galargh deleted the feat/t0116-gateway-cache.sh branch June 28, 2023 11:10
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.

None yet

2 participants