Skip to content

SANDBOX-1889: update e2e wait helper default to PhoneLookupModeDisabled#1292

Merged
fbm3307 merged 3 commits into
codeready-toolchain:masterfrom
fbm3307:SANDBOX-1889-phone-lookup-default
Jul 10, 2026
Merged

SANDBOX-1889: update e2e wait helper default to PhoneLookupModeDisabled#1292
fbm3307 merged 3 commits into
codeready-toolchain:masterfrom
fbm3307:SANDBOX-1889-phone-lookup-default

Conversation

@fbm3307

@fbm3307 fbm3307 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Update UntilToolchainConfigHasPhoneLookupMode to treat nil as PhoneLookupModeDisabled instead of PhoneLookupModeLog.

Related Prs
API: codeready-toolchain/api#515
host-operator: codeready-toolchain/host-operator#1274

registration service: codeready-toolchain/registration-service#604

Assisted By : Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Updated phone lookup mode checks to treat an unset value as disabled by default.
    • Improved the reported differences when the current setting does not match the expected value.

Update UntilToolchainConfigHasPhoneLookupMode to treat nil as
PhoneLookupModeDisabled instead of PhoneLookupModeLog.

Co-authored-by: Cursor <cursoragent@cursor.com>
@openshift-ci openshift-ci Bot requested review from rajivnathan and xcoulon July 6, 2026 10:11
@openshift-ci openshift-ci Bot added the approved label Jul 6, 2026
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Walkthrough

The change updates a test-support helper function so that a nil PhoneLookupMode field defaults to PhoneLookupModeDisabled instead of PhoneLookupModeLog, affecting both the match comparison logic and the generated diff message.

Changes

Phone lookup mode default update

Layer / File(s) Summary
Update default fallback mode
testsupport/wait/host.go
Changes the fallback value for nil PhoneLookupMode from PhoneLookupModeLog to PhoneLookupModeDisabled in both Match logic and Diff output computation.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested labels: test

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: the e2e wait helper now defaults nil phone lookup mode to PhoneLookupModeDisabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, fbm3307, MatousJobanek, metlos

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:
  • OWNERS [MatousJobanek,alexeykazakov,fbm3307,metlos]

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

@fbm3307

fbm3307 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

playwright download issue, in dashboard tests, normal e2e pass

@fbm3307

fbm3307 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e

@fbm3307

fbm3307 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e
fixed the playright dependecy error #1296

@sonarqubecloud

Copy link
Copy Markdown

@fbm3307 fbm3307 merged commit 7ee1873 into codeready-toolchain:master Jul 10, 2026
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved test Work that adds, fixes, or maintains automated tests or coverage (unit, integration, e2e, flakiness)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants