Skip to content

Conversation

@mfrancisc
Copy link
Contributor

@mfrancisc mfrancisc commented Nov 11, 2025

e2e tests for: codeready-toolchain/registration-service#558

Jira: https://issues.redhat.com/browse/SANDBOX-1484

Summary by CodeRabbit

  • New Features

    • Added a webhook URL configuration option for the registration service.
  • Dependencies

    • Bumped the external API module to a newer version.
  • Tests

    • Added end-to-end validation for the UI/configuration endpoint to assert the webhook URL is exposed.

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

Adds workatoWebHookURL to the ToolchainConfig CRD at spec.host.registrationService. Bumps the github.com/codeready-toolchain/api module version in go.mod. Introduces a new e2e test that validates the /api/v1/uiconfig response includes the webhook URL (test is duplicated).

Changes

Cohort / File(s) Summary
Configuration
deploy/host-operator/e2e-tests/toolchainconfig.yaml
Adds spec.host.registrationService.workatoWebHookURL field to ToolchainConfig YAML.
Dependency
go.mod
Updates github.com/codeready-toolchain/api dependency version.
Tests
test/e2e/parallel/registration_service_test.go
Adds TestUIConfig() to exercise /api/v1/uiconfig and assert workatoWebHookURL; the test appears twice (duplicate).

Sequence Diagram(s)

sequenceDiagram
  participant Test as e2e test (client)
  participant Reg as registration-service
  participant Config as ToolchainConfig store

  rect rgba(150, 200, 255, 0.12)
    Test->>Reg: GET /api/v1/uiconfig (signed token)
    note right of Reg: reads configuration
    Reg->>Config: fetch spec.host.registrationService
    Config-->>Reg: returns workatoWebHookURL
    Reg-->>Test: 200 OK with { workatoWebHookURL: "<value>", ... }
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention required:
    • Remove the duplicated TestUIConfig() declaration in test/e2e/parallel/registration_service_test.go.
    • Ensure the new workatoWebHookURL field is surfaced where the registration service reads ToolchainConfig (verify JSON/YAML paths and defaults).
    • Confirm the updated github.com/codeready-toolchain/api version is compatible with existing types used by the registration service.

Suggested reviewers

  • xcoulon
  • jrosental
  • alexeykazakov
  • MatousJobanek
  • rsoaresd

Poem

🐰 A tiny config hop, a webhook new,

I tuck the URL into the queue,
Tests peek in, they chirp and cheer,
One duplicate must disappear,
Now off I bound—deploys are near! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for returning the Workato webhook URL in the /uiconfig endpoint response, which aligns with the test additions and configuration updates.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c47c1d and 32be585.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • deploy/host-operator/e2e-tests/toolchainconfig.yaml (1 hunks)
  • go.mod (1 hunks)
  • test/e2e/parallel/registration_service_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-01T09:54:02.406Z
Learnt from: fbm3307
Repo: codeready-toolchain/toolchain-e2e PR: 1175
File: deploy/base1ns-gotemplate/cluster.yaml:69-70
Timestamp: 2025-08-01T09:54:02.406Z
Learning: The codeready-toolchain/toolchain-e2e project is not using the latest version of Kubernetes yet, so deprecated API groups like `ingresses.extensions` may still be functional in their current environment. Consider this when reviewing resource quotas and API usage.

Applied to files:

  • go.mod
🧬 Code graph analysis (1)
test/e2e/parallel/registration_service_test.go (2)
testsupport/init.go (1)
  • WaitForDeployments (191-256)
testsupport/regsvc.go (1)
  • NewHTTPRequest (18-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: GolangCI Lint
  • GitHub Check: Unit Tests
  • GitHub Check: Build & push operator bundles for e2e tests
  • GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (3)
go.mod (1)

4-4: LGTM! Dependency update aligns with new CRD field.

The API dependency update is necessary to support the new workatoWebHookURL field being added to the ToolchainConfig CRD.

deploy/host-operator/e2e-tests/toolchainconfig.yaml (1)

20-20: LGTM! Test configuration for webhook URL.

The workatoWebHookURL configuration is correctly placed under spec.host.registrationService and uses an appropriate test value for e2e testing.

test/e2e/parallel/registration_service_test.go (1)

993-1020: No duplicate test function declarations found — code is correct.

Verification confirms only one TestUIConfig function declaration exists in the file (line 993). The AI-generated summary was incorrect; there are no duplicates and no compilation errors from this concern.

Copy link
Contributor

@alexeykazakov alexeykazakov left a comment

Choose a reason for hiding this comment

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

Besides the coderebit comment looks good.

@openshift-ci
Copy link

openshift-ci bot commented Nov 11, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexeykazakov, mfrancisc

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 [alexeykazakov,mfrancisc]

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

@sonarqubecloud
Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/e2e/parallel/registration_service_test.go (1)

1006-1008: Remove misleading comment.

The comment states "we have a user in the system" but no user is actually created or required for this test. This appears to be leftover from copying the test structure from TestUsernames above. Since the /uiconfig endpoint is testing configuration values rather than user-specific data, this comment should be removed or updated to reflect the actual test behavior.

Apply this diff to remove the misleading comment:

 	t.Run("get uiconfig 200 response", func(t *testing.T) {
-		// given
-		// we have a user in the system
-
 		// when
 		// we call the get uiconfig endpoint to get ui configuration
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 32be585 and 377a402.

📒 Files selected for processing (1)
  • test/e2e/parallel/registration_service_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/e2e/parallel/registration_service_test.go (2)
testsupport/init.go (1)
  • WaitForDeployments (191-256)
testsupport/regsvc.go (1)
  • NewHTTPRequest (18-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
  • GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (2)
test/e2e/parallel/registration_service_test.go (2)

1016-1016: Previous critical issue has been resolved.

The type assertion on this line is now correct (asserting to string). This resolves the critical issue flagged in the previous review where it was incorrectly asserting to map[string]interface{}.


993-1020: No duplicate test exists — original review comment was based on incorrect information.

The verification confirms only one TestUIConfig function declaration exists in the file (at line 993). The AI-generated summary that prompted the review comment was inaccurate. The code requires no changes.

Likely an incorrect or invalid review comment.

@mfrancisc mfrancisc merged commit 5f7c54b into codeready-toolchain:master Nov 12, 2025
12 of 13 checks passed
@mfrancisc mfrancisc deleted the returnworkatoconfig branch November 12, 2025 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants