-
Notifications
You must be signed in to change notification settings - Fork 1
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
Kind integration feature #71
Conversation
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.
Just one bikeshed-y thing but I'm happy with how small this change is!
Cargo.toml
Outdated
@@ -8,6 +8,7 @@ default-run = "divviup_api_bin" | |||
[features] | |||
default = [] | |||
aggregator-api-mock = [] | |||
kind-integration = [] |
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.
"Running it in Kind" is a shorthand for what we (Divvi Up) would use this for, but I think we should name this so it better captures what it does. How about stub-api-integrations
?
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 doesn't actually stub api integrations, though. It's more like "integration test mode" — the http clients are still configured to talk to the external internet, we just don't add a queue job to do any of those interactions
With this change, I can run
👍🏻 works like a charm! |
b573743
to
db08b67
Compare
We could also drop the content-type and accept requirements with this feature to make things more concise |
I don't think that's necessary. Whatever client we use to hit the divviup-api API in the integration test is perfectly capable of handling those headers correctly. |
@jbr Should we take this change today? It'd unblock some work (#72, https://github.com/divviup/janus-ops/pull/689) that I'd like to land before I go on vacation next week. |
based on #67no longer based on #67