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/19 00:00:02 UTC
[GitHub] [camel-k] u5surf opened a new pull request, #3828: chore(test) : TestRunAndLog to pass on mac os
u5surf opened a new pull request, #3828:
URL: https://github.com/apache/camel-k/pull/3828
* Fix: #3813
Signed-off-by: yugo-horie <u5...@gmail.com>
<!-- Description -->
#3813
We divided the test case each OS.
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
TestRunAndLog to pass both linux and MacOS
```
--
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 a diff in pull request #3828: chore(test) : TestRunAndLog to pass on mac os
Posted by GitBox <gi...@apache.org>.
u5surf commented on code in PR #3828:
URL: https://github.com/apache/camel-k/pull/3828#discussion_r1030979575
##########
pkg/util/command_test.go:
##########
@@ -56,5 +57,9 @@ func TestRunAndLogInvalid(t *testing.T) {
err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError)
assert.NotNil(t, err)
- assert.ErrorContains(t, err, "exit status 1")
+ if runtime.GOOS == "darwin" {
+ assert.ErrorContains(t, err, "exit status 1")
Review Comment:
@squakez
> darwin (we should understand which is the output there)
It failed in my macOS environment when I changed the test below.
And it had not the difference between aarch64 and amd64.
```
diff --git a/pkg/util/command_test.go b/pkg/util/command_test.go
index 38c08c367..9b4498343 100644
--- a/pkg/util/command_test.go
+++ b/pkg/util/command_test.go
@@ -58,7 +58,7 @@ func TestRunAndLogInvalid(t *testing.T) {
assert.NotNil(t, err)
if runtime.GOOS == "darwin" {
- assert.ErrorContains(t, err, "exit status 1")
+ assert.ErrorContains(t, err, "date: illegal option -- d")
} else {
assert.ErrorContains(t, err, "date: invalid date ‘sa’:")
}
```
test result
```
--- FAIL: TestRunAndLogInvalid (0.03s)
command_test.go:61:
Error Trace: /Users/yugo-horie/camel-k/pkg/util/command_test.go:61
Error: Error ": exit status 1" does not contain "date: illegal option -- d"
Test: TestRunAndLogInvalid
FAIL
FAIL github.com/apache/camel-k/pkg/util 0.494s
```
And moreover linux date error messages seems to be different by each environments because
my ubuntu20.04 indicate the following message which doesn't including command full path. Thus
I consider that the it doesn't need the assert which is including the full path of the command in error messages.
--
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 pull request #3828: chore(test) : TestRunAndLog to pass on mac os
Posted by GitBox <gi...@apache.org>.
squakez commented on PR #3828:
URL: https://github.com/apache/camel-k/pull/3828#issuecomment-1331942086
Thanks!
--
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 a diff in pull request #3828: chore(test) : TestRunAndLog to pass on mac os
Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3828:
URL: https://github.com/apache/camel-k/pull/3828#discussion_r1027716411
##########
pkg/util/command_test.go:
##########
@@ -56,5 +57,9 @@ func TestRunAndLogInvalid(t *testing.T) {
err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError)
assert.NotNil(t, err)
- assert.ErrorContains(t, err, "exit status 1")
+ if runtime.GOOS == "darwin" {
+ assert.ErrorContains(t, err, "exit status 1")
Review Comment:
I'd prefer to have a switch here to test linux ("date: invalid date ‘sa’"), darwin (we should understand which is the output there) and else any other platform (ie, windows, with generic "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
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a diff in pull request #3828: chore(test) : TestRunAndLog to pass on mac os
Posted by GitBox <gi...@apache.org>.
squakez commented on code in PR #3828:
URL: https://github.com/apache/camel-k/pull/3828#discussion_r1031209420
##########
pkg/util/command_test.go:
##########
@@ -56,5 +57,9 @@ func TestRunAndLogInvalid(t *testing.T) {
err := RunAndLog(context.Background(), cmd, loggerInfo, loggerError)
assert.NotNil(t, err)
- assert.ErrorContains(t, err, "exit status 1")
+ if runtime.GOOS == "darwin" {
+ assert.ErrorContains(t, err, "exit status 1")
Review Comment:
Interesting. For some reason, in `darwin` we are not returning the error trace as it happens in `linux`. The test was good, but the failure shows us that maybe the implementation does not do what it is expected to do. If you notice the execution of the command returns `: exit status 1`, like it miss to capture the expected output `date: illegal option -- d`.
--
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 merged pull request #3828: chore(test) : TestRunAndLog to pass on mac os
Posted by GitBox <gi...@apache.org>.
squakez merged PR #3828:
URL: https://github.com/apache/camel-k/pull/3828
--
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