Test framework #28
Test framework #28
Conversation
mike-sul
force-pushed
the
test-framework
branch
3 times, most recently
from
bfb433b
to
347f29a
Compare
test/integration/smoke_test.go
Outdated
| c.Dir = appDir | ||
| output, err := c.CombinedOutput() | ||
| if err != nil { | ||
| t.Errorf("failed to run `%s` command: %s\n", args[0], output) |
There was a problem hiding this comment.
You probably want t.Fatal here or your test is just going to fail everything after this. It's also going to get tedious checking for errors everywhere. The require.Nil package/function makes this a lot easier
There was a problem hiding this comment.
I get the feeling all your tests are going to need this function. I'm guessing the code is going to be easier if you make this a function
There was a problem hiding this comment.
Let me restructure the PR so the fixtures appear at first and then the test cases. Probably, after that the comments will become irrelevant.
There was a problem hiding this comment.
The require.Nil package/function makes this a lot easier
Agree, the simple function should do the trick, not sure it is worth pulling another deps for it though.
There was a problem hiding this comment.
@doanac I've restructured commits in the PR. Please, review it now.
I suppose it makes sense from the test case perspective, and a commit history will be cleaner. I will update PR accordingly. (I just wanted to justify the fixtures and how they evolutionary appears) |
mike-sul
force-pushed
the
test-framework
branch
2 times, most recently
from
a8fd8e5
to
c786354
Compare
| @@ -0,0 +1,31 @@ | |||
| -----BEGIN CERTIFICATE----- | |||
There was a problem hiding this comment.
You might include the script to generate this. You have the conf file. I remember using something like this from moby once and losing 15 minutes needing to generate a cert.
There was a problem hiding this comment.
Add a compose-based environment to build, run, and test the composectl utility. The environment includes the foundries specific docker registry and daemon (vanilla version + required patches). Signed-off-by: Mike Sul <[email protected]>
Signed-off-by: Mike Sul <[email protected]>
Add tests for the following cases: 1. The same image is used by more than one compose service. 2. There are more than one version of the same app in the app store. Signed-off-by: Mike Sul <[email protected]>
Signed-off-by: Mike Sul <[email protected]>
mike-sul
force-pushed
the
test-framework
branch
from
c786354
to
8bba07d
Compare
|
Perhaps adding a comment to the README.md mentioning how to execute the e2e tests locally would be helpful, which seems to be: Other then that, LGTM. |
It makes sense, I'll update the readme. |
Signed-off-by: Mike Sul <[email protected]>
No description provided.