You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "realMartinez (via GitHub)" <gi...@apache.org> on 2023/04/04 11:42:50 UTC
[GitHub] [camel-k] realMartinez opened a new pull request, #4228: chore: keystore_test file path change
realMartinez opened a new pull request, #4228:
URL: https://github.com/apache/camel-k/pull/4228
<!-- Description -->
- changed file path of pkg/util/jvm/keystore_test files to /tmp/ directory
- added test file cleanup
<!--
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
NONE
```
--
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] gansheer commented on a diff in pull request #4228: chore: keystore_test file path change
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#discussion_r1157396644
##########
pkg/util/jvm/keystore_test.go:
##########
@@ -29,15 +31,20 @@ func TestGenerateKeystore(t *testing.T) {
// No Data
var data [][]byte
ctx := context.Background()
- err := GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err := GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.Nil(t, err)
// Correct input
data = [][]byte{{0}, {1}}
- err = GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err = GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.NotNil(t, err)
Review Comment:
I know it is out if scope, but I am really confused about the fact that we expect an error on something called "Correct input".
--
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 #4228: chore: keystore_test file path change
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#discussion_r1158130287
##########
pkg/util/jvm/keystore_test.go:
##########
@@ -29,15 +31,20 @@ func TestGenerateKeystore(t *testing.T) {
// No Data
var data [][]byte
ctx := context.Background()
- err := GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err := GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.Nil(t, err)
// Correct input
data = [][]byte{{0}, {1}}
- err = GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err = GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.NotNil(t, err)
Review Comment:
Yes, no rush 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] gansheer commented on a diff in pull request #4228: chore: keystore_test file path change
Posted by "gansheer (via GitHub)" <gi...@apache.org>.
gansheer commented on code in PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#discussion_r1157396644
##########
pkg/util/jvm/keystore_test.go:
##########
@@ -29,15 +31,20 @@ func TestGenerateKeystore(t *testing.T) {
// No Data
var data [][]byte
ctx := context.Background()
- err := GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err := GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.Nil(t, err)
// Correct input
data = [][]byte{{0}, {1}}
- err = GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err = GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.NotNil(t, err)
Review Comment:
I know it is out if scope, but I am really confused about the fact that we expect an error on something called "Correct input". It is not something that needs to be fixed, merely an observation.
--
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] realMartinez commented on pull request #4228: chore: keystore_test file path change
Posted by "realMartinez (via GitHub)" <gi...@apache.org>.
realMartinez commented on PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#issuecomment-1495829788
this is a commit resolving issue #4217
--
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 #4228: chore: keystore_test file path change
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #4228:
URL: https://github.com/apache/camel-k/pull/4228
--
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 #4228: chore: keystore_test file path change
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#discussion_r1157426578
##########
pkg/util/jvm/keystore_test.go:
##########
@@ -29,15 +31,20 @@ func TestGenerateKeystore(t *testing.T) {
// No Data
var data [][]byte
ctx := context.Background()
- err := GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err := GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.Nil(t, err)
// Correct input
data = [][]byte{{0}, {1}}
- err = GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err = GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.NotNil(t, err)
Review Comment:
I guess the comment is unfortunate and it should have been something like non-nil data.
--
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] realMartinez commented on a diff in pull request #4228: chore: keystore_test file path change
Posted by "realMartinez (via GitHub)" <gi...@apache.org>.
realMartinez commented on code in PR #4228:
URL: https://github.com/apache/camel-k/pull/4228#discussion_r1158122493
##########
pkg/util/jvm/keystore_test.go:
##########
@@ -29,15 +31,20 @@ func TestGenerateKeystore(t *testing.T) {
// No Data
var data [][]byte
ctx := context.Background()
- err := GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err := GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.Nil(t, err)
// Correct input
data = [][]byte{{0}, {1}}
- err = GenerateKeystore(ctx, "", "name", NewKeystorePassword(), data)
+ err = GenerateKeystore(ctx, "", "/tmp/keystore", NewKeystorePassword(), data)
assert.NotNil(t, err)
Review Comment:
I will take a look at that later this week, the comment is a leftover. Can create another PR with it rephrased
--
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