You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "christophd (via GitHub)" <gi...@apache.org> on 2024/02/27 11:07:03 UTC
[PR] WIP: chore(e2e): Improve E2E tests [camel-k]
christophd opened a new pull request, #5188:
URL: https://github.com/apache/camel-k/pull/5188
(no comment)
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985108943
Documentation to reflect the changes in https://camel.apache.org/camel-k/2.2.x/contributing/e2e.html would be more than welcome.
--
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
Re: [PR] WIP: chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1512342932
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
I would not put too much effort into fixing this on the kamel CLI as it may be fixed already when using the Camel K JBang plugin instead
--
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
Re: [PR] WIP: chore(e2e): Improve E2E tests [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1512280139
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
Wonder if the culprit is the fact that we are using a shared viper instance instead of using one per command instance
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985106790
> @squakez I tried to run the native tests locally which ends up in timeouts while building the integrations. I tested this also on the main branch with the same result. So I guess my local setup is not ready to run these native E2E tests. Do you have any working setup or information how to properly run the tests locally?
Minikube with 16 GB or RAM and 4 CPUs should be good. At least, it works for me.
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1515799045
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
no offense to you the viper library totally offers the shared usage as a valid option. it took me some time to really understand why the tests hit the concurrency error 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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1984543351
@squakez I tried to run the native tests locally which ends up in timeouts while building the integrations. I tested this also on the main branch with the same result. So I guess my local setup is not ready to run these native E2E tests. Do you have any working setup or information how to properly run the tests locally?
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1515787122
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
@lburgazzoli BTW I ended up fixing the shared Viper instance so each command uses its own instance. Works like a charm now without the need for the lock on the mutex.
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1983845623
I've enabled Quarkus Native tests. However it's very likely they fail due to some infrastructure problem that we have not yet tackled. In such circumstances, please, run the Quarkus Native test manually in your local environment and report result here.
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985466766
> @squakez I tried to setup native e2e tests locally but did not manage to get it working on my machine. I don't think that my changes changed the way we build and run native images. Do you approve to merge?
>
> Or if you have a running env for native e2e tests could you please checkout my branch and run the tests? 🙏
I think it would be convenient you manage to find the proper setup, also because you may need to run this in future while working on any other project feature. Running these tests unfortunately takes a long amount of time and tends to lock the resource. So, it can be a bottleneck to the normal working stream if one run them in parallel. I will try to checkout this PR and run the test at the end of my working day for this occasion.
As I said in a separate stream, we should work to find a solution and have the GH actions back working. It's something we know since enough time now and we all need to put particular attention to the normal maintenance of the project, not exclusively to new developments or workstream required to enable some marginal feature.
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985447310
@squakez I tried to setup native e2e tests locally but did not manage to get it working on my machine. I don't think that my changes changed the way we build and run native images. Do you approve to merge?
Or if you have a running env for native e2e tests could you please checkout my branch and run the tests? 🙏
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd merged PR #5188:
URL: https://github.com/apache/camel-k/pull/5188
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1986392093
Forget my last comment, I see the `main` branch is up to date. 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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1986295005
@squakez many thanks. I managed to get a setup working on my arm64 machine with the native E2E tests. Run them all manually and all have been green! Will continue to merge now
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1986390359
> @squakez many thanks. I managed to get a setup working on my arm64 machine with the native E2E tests. Run them all manually and all have been green! Will continue to merge now
Good. Do you mind also updating the documentation [1] with all the latest changes? thanks.
[1] https://camel.apache.org/camel-k/2.2.x/contributing/e2e.html
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1515792033
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
very good, you cna probably blame me for that original design :)
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
very good, you can probably blame me for that original design :)
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985141422
> Minikube with 16 GB or RAM and 4 CPUs should be good. At least, it works for me.
Hm, quite heavy setup for me as I am running on MacOS and need to run this in Docker desktop that also requires quite some memory
--
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
Re: [PR] WIP: chore(e2e): Improve E2E tests [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1512280139
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
Wonder if the culprit is the fact that we are using a shared viper instance instead of using one per command instance (which may not be super easy to get rid of)
--
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
Re: [PR] WIP: chore(e2e): Improve E2E tests [camel-k]
Posted by "lburgazzoli (via GitHub)" <gi...@apache.org>.
lburgazzoli commented on code in PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#discussion_r1512359296
##########
pkg/cmd/root.go:
##########
@@ -93,6 +97,10 @@ func kamelPreAddCommandInit(options *RootCmdOptions) *cobra.Command {
}
func kamelPostAddCommandInit(cmd *cobra.Command) error {
+ // Requires synchronization as viper bind flag is not able to handle concurrency
+ m.Lock()
+ defer m.Unlock()
+
Review Comment:
100% agree, it was more a consideration about the original design
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985810056
@squakez rebase done
--
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
Re: [PR] chore(e2e): Improve E2E tests [camel-k]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on PR #5188:
URL: https://github.com/apache/camel-k/pull/5188#issuecomment-1985716413
@christophd please, rebase to main, and once ready let me know so I can run the test locally
--
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