Skip to content

test : fix flaky failures in sync_test by making sure non-empty string pointer is generated#1464

Merged
dkwon17 merged 1 commit intodevfile:mainfrom
rohankanojia-forks:pr/issue1443
Jul 28, 2025
Merged

test : fix flaky failures in sync_test by making sure non-empty string pointer is generated#1464
dkwon17 merged 1 commit intodevfile:mainfrom
rohankanojia-forks:pr/issue1443

Conversation

@rohanKanojia
Copy link
Copy Markdown
Member

@rohanKanojia rohanKanojia commented Jul 7, 2025

What does this PR do?

Previously in fuzzStringPtr, we were skipping assigning the value of the generated string was empty, meaning str remained nil. This was causing flaky behavior in the test as mergeConfig logic checks if the pointer is non-nil before merging a value.

The updated fuzzStringPtr function ensures that the pointer is always set and never set to an empty string. It guarantees that all fuzzed *string fields are non-nil and non-empty, ensuring mergeConfig includes them during merging.

We have a test for Proxy field values to cover nil proxy value scenarios:

func TestMergeConfigHandlesProxySettings(t *testing.T) {

I've added similar tests for other *string fields like StorageClassName and RuntimeClassName

What issues does this PR fix or reference?

Fix #1443

Is it tested? How?

I ran this test over 50 times, and it seems to pass on all attempts with these changes.

As per old changes, I saw 8 failures on 50 runs.

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

…g pointer is generated

Previosly in `fuzzStringPtr`, we were skipping assigning value of the generated string was empty.
This was causing flaky behavior in test as mergeConfig logic checks if pointer is non-nil before
merging a value.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 7, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rohanKanojia
Copy link
Copy Markdown
Member Author

/ok-to-test

@rohanKanojia rohanKanojia marked this pull request as ready for review July 7, 2025 19:02
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dkwon17, rohanKanojia

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

The pull request process is described 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

@dkwon17 dkwon17 merged commit 79c2242 into devfile:main Jul 28, 2025
10 checks passed
@dkwon17
Copy link
Copy Markdown
Collaborator

dkwon17 commented Jul 28, 2025

Thank you!

@rohanKanojia rohanKanojia deleted the pr/issue1443 branch July 28, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sync_test.go is intermittently failing sometimes

2 participants