-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feat/e2e release tests #221
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.
Great job on this, also love the fact that we finally (hopefully) tied down the issues we had with gojson
too.
|
||
// Helper function to run docker command | ||
// This assumes that docker assets are built (make all) and integration resources exist (make integration-up) | ||
func runDockerCommand(secondsBeforeShutdown time.Duration, testName string, configFilePath string, binaryVersion string, additionalOpts string) ([]byte, error) { |
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.
Is there a reason why you chose to use Docker using bash instead of using a package such as https://pkg.go.dev/github.com/docker/docker/client#pkg-index?
Although I'm unsure if it might be worth importing this package just for this.
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.
Honest answer is I started with a janky bash script to spike things out, and I never thought of this alternative way.
I'll take a look at what using the package would look like
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.
Looks great @colmsnowplow .
release_test/e2e_source_test.go
Outdated
return strings.Split(string(inputData), "\n") | ||
} | ||
|
||
func TestE2EPubsubSource(t *testing.T) { |
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.
If you run all possible (source, target) combos, you won't need a separate suit of source tests, as the sources will be tested as part of the combo.
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 think having them separate is advantageous from a debugging point of view - I'd like it to be unambiguous where the problem is.
However, I do think we can extend the tests to combine sources, targets and transformations. I didn't go that far in this PR purely from the POV of keeping it simple and delivering a first iteration.
I'd like to do a bit of scoping work to decide which combinations to test (maybe all sources & targets, maybe all combinations of all 3, maybe just key use cases. Not sure), and deliver something along those lines in a future release.
e871d87
to
6da7734
Compare
fe5256b
to
922ecd4
Compare
This PR adds end-to-end release testing for stream replicator. This consists of a set of tests which run against the docker asset itself, using predefined configurations.
Resources are emulated via localstack, pubsub emulator, a http server built in-code, and a kafka instance. This is done with a view to using full resource deployments in future.
There are no EventHub tests becuase no local emulator exists for EventHubs.
Additionally:
#215
#212
#214
The latter of these means that some Lua transformation modes are untested in these e2e tests - we will address this in the same release, however.
Commands (from repo root):
First build the docker container locally with
make all
.Setup test resources:
make e2e-up
Run the release tests:
make e2e-test
Tear down test resources:
make e2e-down