Conversation
d747b1e to
def6d16
Compare
This should test on plugin and standalone mode Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
def6d16 to
a97a736
Compare
pkg/e2e/main_test.go
Outdated
|
|
||
| func TestMain(m *testing.M) { | ||
| logrus.Info("Running tests on COMPOSE_STANDALONE_MODE=true") | ||
| _ = os.Setenv(composeStandaloneEnvVar, "true") |
There was a problem hiding this comment.
Can’t we do this using a command line flag and a (global) variable instead?
There was a problem hiding this comment.
My second option was to run the tests command line 2 times. One for each configuration of the envvar.
I'm not sure if I understood what you mean by using a global variable for this.
There was a problem hiding this comment.
Why do you need an env var at all? Is this not the same process?
There was a problem hiding this comment.
Ahhh... Finally got what you mean. The idea was to have it also configurable from outside of this TestMain.
With the envvar we can run just one specific test. And when running like this, TestMain doesn't run. So the envvar is a solution to be able to still pass this info to the test.
There was a problem hiding this comment.
But can’t we use a command line flag instead?
Sorry it’s just I don’t like env vars much, they tend to magically configure (break) things, a bit like global vars on steroids 😀
There was a problem hiding this comment.
The only alternative I see is to pass a build tag when running the test to choose in between tagged files. So we can choose which file to use, so we can use that conditionally depending on the tag.... But that really looks like over engineering and gets to the same problematic in the end...
|
🏆 |
pkg/e2e/framework.go
Outdated
| const composeStandaloneEnvVar = "COMPOSE_STANDALONE_MODE" | ||
|
|
||
| // RunDockerComposeCmd runs a docker compose command, expects no error and returns a result | ||
| func (c *E2eCLI) RunDockerComposeCmd(args ...string) *icmd.Result { | ||
| composeStandaloneMode := os.Getenv(composeStandaloneEnvVar) | ||
| if composeStandaloneMode == "true" || composeStandaloneMode == "1" { |
There was a problem hiding this comment.
| const composeStandaloneEnvVar = "COMPOSE_STANDALONE_MODE" | |
| // RunDockerComposeCmd runs a docker compose command, expects no error and returns a result | |
| func (c *E2eCLI) RunDockerComposeCmd(args ...string) *icmd.Result { | |
| composeStandaloneMode := os.Getenv(composeStandaloneEnvVar) | |
| if composeStandaloneMode == "true" || composeStandaloneMode == "1" { | |
| composeStandaloneMode := false | |
| // RunDockerComposeCmd runs a docker compose command, expects no error and returns a result | |
| func (c *E2eCLI) RunDockerComposeCmd(args ...string) *icmd.Result { | |
| if composeStandaloneMode { |
then in the main:
func TestMain(m *testing.M) {
logrus.Info("Running tests on COMPOSE_STANDALONE_MODE=true")
composeStandaloneMode = true
exitCode := m.Run()
if exitCode != 0 {
os.Exit(exitCode)
}
logrus.Info("Running tests on COMPOSE_STANDALONE_MODE=false")
composeStandaloneMode = false
os.Exit(m.Run())
}
We don’t really need an env var, do we?
There was a problem hiding this comment.
would be nice this can be set as a go test argument so that we can parallelize run on CI
There was a problem hiding this comment.
Not sure if parallelising this would be possible...
The e2e tests open ports and maybe that would conflict with it's twin test
There was a problem hiding this comment.
Also, I don't see a good way to run one specific test (by passing E2E_TEST environment variable) on one specific mode and not having to replicate Makefile targets or something like that...
There was a problem hiding this comment.
this is what I had in mind: two targets, two parallel GHA workfows
never mind
There was a problem hiding this comment.
What’s a Makefile? 😬
It’s already possible to focus tests out of the box using -run with go test: https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks
(I don’t have make on my machines)
There was a problem hiding this comment.
@mat007 Yep. And that's what the Makefile does. The thing is, when targeting a specific test TestMain doesn't run. So that would execute just one of the modes (the default one).
Then the question is... Do we separate the execution of the 2 modes into different runs or leave it in the same (like it's implemented in this PR).
Now I'm leaning to having 2 runs in the GHA as @ndeloof said cuz that would be easier to run in local. Cuz I don't want to run the 2 things all the time I test locally. But for the CI that's important. And also that can be run in parallel...
|
@mat007 PTAL |
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
6a4b2f2 to
8ae8d99
Compare
What I did
This should test on plugin and standalone mode