You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/11/15 10:01:04 UTC
[GitHub] [camel-k] squakez opened a new issue, #3813: TestRunAndLog refactoring
squakez opened a new issue, #3813:
URL: https://github.com/apache/camel-k/issues/3813
With https://github.com/apache/camel-k/pull/3805/commits/98cf97fb780663430772d6a6317e7e695e3aa551 we have added a fix for a wrong behavior on non Linux OS. However, we're not testing what we expect to test as we need to check an additional message beside the `exit status 1`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez closed issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
squakez closed issue #3813: TestRunAndLog refactoring
URL: https://github.com/apache/camel-k/issues/3813
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] u5surf commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
u5surf commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1328157135
@squakez
Hi, I changed the test cases what you said, but It seems to be failed.
```
diff --git a/pkg/util/command_test.go b/pkg/util/command_test.go
index 98187b097..d68a2e774 100644
--- a/pkg/util/command_test.go
+++ b/pkg/util/command_test.go
@@ -52,9 +52,9 @@ func TestRunAndLog(t *testing.T) {
}
func TestRunAndLogInvalid(t *testing.T) {
- cmd := exec.CommandContext(context.Background(), "date", "-dsa")
+ cmd := exec.CommandContext(context.Background(), "go", "help", "dsa")
err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError)
assert.NotNil(t, err)
- assert.ErrorContains(t, err, "exit status 1")
+ assert.Equal(t, "go help dsa: unknown help topic. Run 'go help'.: exit status 2", err.Error())
}
```
If we check only phrase of the `: exit status 2`, we can check by assert.ErrorContains, but
I'm confusing that it doen't satisfy what you said that "we're not testing what we expect to test as we need to check an additional message beside the exit status 1."
```
--- FAIL: TestRunAndLogInvalid (0.00s)
command_test.go:59:
Error Trace: /home/u5surf/camel-k/pkg/util/command_test.go:59
Error: Not equal:
expected: "go help dsa: unknown help topic. Run 'go help'.: exit status 2"
actual : ": exit status 2"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-go help dsa: unknown help topic. Run 'go help'.: exit status 2
+: exit status 2
Test: TestRunAndLogInvalid
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] u5surf commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
u5surf commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1315350464
@squakez
Hi, I'm a newbie of this repos, but I'd like to work on this issue. I consider that the issue can be fix to change the asserting the error message by each OS. Could you correct if I'm saying something wrong?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
squakez commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1326141521
I think I have a "universal command" that must be present on the machine we're using to test... it's `go`!
We can try to test the following on any machine:
```
$ go help dsa
go help dsa: unknown help topic. Run 'go help'.
$ echo $?
2
```
This command should return an error (code 2) and a log trace that we should be able to capture. @u5surf do you think you can change the usage of `date` with the above one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
squakez commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1315373349
Thanks for taking interest in this. Well, it could be a solution. As alternative we may find any command that return the same output error for the most of OS existing (if any exists, indeed...) and check it. Feel free to submit a PR with your ideas, though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
squakez commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1328717083
So, the test you wrote is correct. It proves however that the implementation of `RunAndLog` func is wrong. If you run it, you will see the output:
```
OUT: Executed command: /usr/local/go/bin/go help dsa
ERR: go help dsa: unknown help topic. Run 'go help'.
```
The `stderr` contains the trace that we should include in our `RunAndLog` error returned, but we're not including it for some reason. We should troubleshoot the func against this unit test to fix it accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] u5surf commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
u5surf commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1326432002
Indeed, I also consider that it makes a good solution. As an aside, we found the weird behavior that only happen in darwin.
Can you see this review?
https://github.com/apache/camel-k/pull/3828#discussion_r1030979575
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on issue #3813: TestRunAndLog refactoring
Posted by GitBox <gi...@apache.org>.
squakez commented on issue #3813:
URL: https://github.com/apache/camel-k/issues/3813#issuecomment-1326479128
I have seen that, and, I think we should rework entirely the test to use `go` command instead of `date` command. Ideally, it should produce the very same output on all platforms.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org