Skip to content

Add e2e ginkgo tests for extraCommandargs and Application Health Status #923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jgwest merged 1 commit into from
Sep 25, 2025
Merged

Add e2e ginkgo tests for extraCommandargs and Application Health Status #923

jgwest merged 1 commit into from
Sep 25, 2025

Conversation

@Rizwana777
Copy link
Collaborator

@Rizwana777 Rizwana777 commented Jul 18, 2025

e2e test for argoproj-labs/argocd-operator#1763 and argoproj-labs/argocd-operator#1722

Will make few more changes if needed once this PR is merged

@openshift-ci openshift-ci bot requested review from anandrkskd and trdoyle81 July 18, 2025 07:38
@openshift-ci
Copy link

openshift-ci bot commented Jul 18, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign wtam2018 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Rizwana777 Rizwana777 marked this pull request as draft July 18, 2025 07:38
@Rizwana777 Rizwana777 force-pushed the ginkgo-e2e-test branch 2 times, most recently from f1696c0 to e585c73 Compare July 29, 2025 09:02
@Rizwana777 Rizwana777 marked this pull request as ready for review July 29, 2025 09:03
@openshift-ci openshift-ci bot requested a review from varshab1210 July 29, 2025 09:03
@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Jul 29, 2025

/retest

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 14, 2025

/retest

1 similar comment
@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 18, 2025

/retest

jgwest
jgwest requested changes Aug 19, 2025
View reviewed changes
test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go

// Expect default values to be replaced (old default 10 should not appear)
Consistently(func() bool {
cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the top of this Consistently, you will need to call k8sClient.Get(ctx, (...), appControllerSS) to ensure you have the latest copy from K8s. Otherwise you are just comparing the same Go object over and over again (which is not useful)

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed this comment, i have added k8sClient.Get(ctx, (...), appControllerSS) above, https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR104

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go

// But no duplicate --status-processors or --kubectl-parallelism-limit
Consistently(func() bool {
cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, need to k8sClient.Get at top of func

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR137

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

test/openshift/e2e/ginkgo/parallel/1-115_validate_controller_extra_command_args_test.go
// Eventually(appControllerSS).ShouldNot(statefulsetFixture.HaveContainerCommandSubstring("--status-processors 15", 0))
// Check that both --metrics-application-labels flags are present
Eventually(func() bool {
cmd := appControllerSS.Spec.Template.Spec.Containers[0].Command
Copy link
Member

@jgwest jgwest Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, need to k8sClient.Get at top

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/redhat-developer/gitops-operator/pull/923/files#diff-4eeec8e6b675c5a0710f157abb0ba91787c32219b01be28922eb7f8d1d6a138cR173

Copy link
Collaborator Author

@Rizwana777 Rizwana777 Sep 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Rizwana777 Rizwana777 force-pushed the ginkgo-e2e-test branch 2 times, most recently from 5ebd2f4 to 9822507 Compare August 20, 2025 10:19
@jgwest
Copy link
Member

jgwest commented Aug 21, 2025

/retest

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 25, 2025

/retest

@Rizwana777 Rizwana777 force-pushed the ginkgo-e2e-test branch 2 times, most recently from 112e778 to 33347ac Compare August 28, 2025 13:58
@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 28, 2025

/retest

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 28, 2025

/retest

2 similar comments
@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Aug 29, 2025

/retest

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Sep 1, 2025

/retest

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Sep 2, 2025

/retest

2 similar comments
@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Sep 3, 2025

/retest

@jgwest
Copy link
Member

jgwest commented Sep 16, 2025

/retest

@jgwest
Copy link
Member

jgwest commented Sep 16, 2025

Some review comments are still unhandled, as of Sept 16 2025.

@Rizwana777
Copy link
Collaborator Author

Rizwana777 commented Sep 22, 2025

Some review comments are still unhandled, as of Sept 16 2025.

Hi @jgwest I have addressed all the comments, I might have missed something, can you please point me out?

@jgwest
Copy link
Member

jgwest commented Sep 24, 2025

/retest

jgwest
jgwest approved these changes Sep 24, 2025
View reviewed changes
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @Rizwana777!

@jgwest jgwest merged commit 82492ea into redhat-developer:master Sep 25, 2025
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jgwest jgwest jgwest approved these changes

@anandrkskd anandrkskd Awaiting requested review from anandrkskd

@trdoyle81 trdoyle81 Awaiting requested review from trdoyle81

@varshab1210 varshab1210 Awaiting requested review from varshab1210

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Rizwana777 @jgwest