Fix/code flow e2e test#793
Conversation
…itory selection - Fix onboarding page detection by properly waiting for and clicking the onboarding options - Update repository selection to use correct data-testid format (slashes replaced with dashes) - Change repository from wecode-ai/Wegent to weibo_rd/common/wecode/wegent which exists in the test environment - Add better error handling and logging for team, repository, and branch selection - Add verification that repository is actually selected before continuing - Increase wait timeouts for more reliable test execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The onboarding page click was targeting the h3 heading element, but the actual clickable element is the parent card with data-testid. Changed to use data-testid="onboarding-cloud-ide-option" for the Cloud IDE option and data-testid="onboarding-ide-option" for the fallback IDE option. This follows the E2E testing guidelines to prefer data-testid selectors over text content selectors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test was failing because the \"Bug or feature?\" onboarding page was not being properly detected and handled. This commit adds: 1. Multiple click strategies for the onboarding options: - Try clicking the heading element directly - Try clicking parent button element via XPath - Try clicking card div with border/cursor-pointer classes - Fallback to getByText selector 2. Better error handling with try-catch blocks 3. Improved logging to diagnose which strategy worked This ensures the test can proceed past the onboarding page regardless of the exact DOM structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughTest setup enhancements for code-flow testing include improved onboarding detection and handling, more robust selection logic with fallback strategies for team/repository/branch selection, increased timeout consistency, and added validation to verify proper selection state. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b428e81659
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (await wegentRepoOption.isVisible({ timeout: 3000 }).catch(() => false)) { | ||
| // Select weibo_rd/common/wecode/wegent repository - wait for it to appear after search | ||
| // Note: data-testid replaces / with - | ||
| const wegentRepoOption = page.locator('[data-testid="repo-option-weibo_rd-common-wecode-wegent"]').first() |
There was a problem hiding this comment.
This hard-codes repo-option-weibo_rd-common-wecode-wegent, but the test suite still documents and expects wecode-ai/Wegent for code-flow runs. In environments that only expose the documented public repo, this locator never resolves, so setup cannot pick a repository and the test fails before validating any code-flow behavior. Please keep the canonical repo target (or make it configurable per environment) instead of pinning to an internal-only repo id.
Useful? React with 👍 / 👎.
| for (let i = 0; i < count; i++) { | ||
| const text = await repoOptions.nth(i).textContent() | ||
| console.log(`Repo option ${i}: ${text}`) | ||
| if (text && text.includes('wegent')) { |
There was a problem hiding this comment.
The fallback filter uses text.includes('wegent'), which is case-sensitive. If the option text is .../Wegent (capital W), the fallback path won't select any repository when the exact data-testid lookup misses, and setup proceeds with no repo selected. Converting both sides to lowercase before matching avoids this brittle failure mode.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/specs/code-flow.spec.ts`:
- Around line 58-60: The test is locating the onboarding dialog by localized
heading text via onboardingHeading = page.locator('h2', { hasText: ... }) which
is brittle; update the locator to use the stable data-testid attributes for the
onboarding dialog/cards (replace the page.locator call that sets
onboardingHeading and any similar locators in the 65-129 range) so tests target
the onboarding test IDs instead of localized copy or XPath; ensure you update
all occurrences (the onboardingHeading variable and any other page.locator
references for onboarding elements) to use the appropriate data-testid strings
provided in the PR.
- Around line 217-229: The test currently selects the first repo option whose
visible text contains 'wegent', which is ambiguous; change selection to match
the exact data-testid value exposed by RepoListView (the full repo label present
in the repo option data-testid) instead of using text.includes. Locate the
repoOptions locator and replace the fuzzy match loop (using textContent/includes
'wegent') with a check against the exact expected data-testid (e.g., compare
repoOptions.nth(i).getAttribute('data-testid') to the known full repo label) and
click when equal; then assert the selected repo/branch equals that exact
expected label (also update the analogous logic at lines ~284-287) so the test
fails if a sibling nested repo is chosen.
- Around line 73-80: The test currently tries multiple click strategies on
cloudIdeHeading but sleeps on failure; instead, after the click attempts using
cloudIdeHeading and subsequent strategies (the clicked flag), if clicked is
still false throw an error to fail fast and call a visibility wait to ensure the
onboarding modal/dialog is gone (e.g., await
page.waitForSelector('<onboarding-selector>', { state: 'hidden', timeout: 5000
}) or use the existing onboarding dialog locator) before proceeding so
subsequent clicks aren't intercepted; update the block around cloudIdeHeading
and the similar block at lines 123-135 to throw when no option was clicked and
to explicitly wait for the onboarding UI to disappear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa8cca7a-48b2-49f9-8e14-0368e7404f59
📒 Files selected for processing (1)
tests/specs/code-flow.spec.ts
| // Check for the onboarding page by looking for the characteristic heading | ||
| const onboardingHeading = page.locator('h2', { hasText: '选择最适合您的方式继续编码之旅' }).first() | ||
| const isOnboardingVisible = await onboardingHeading.isVisible({ timeout: 5000 }).catch(() => false) |
There was a problem hiding this comment.
Use the onboarding test IDs here instead of localized copy/XPath.
This path still depends on Chinese headings/text and DOM ancestry, so it will miss the dialog in the English UI or after a copy/layout tweak. The onboarding cards already have stable test IDs in the PR context; use those directly and drop the XPath/text fallbacks.
🎯 Suggested change
- const onboardingHeading = page.locator('h2', { hasText: '选择最适合您的方式继续编码之旅' }).first()
- const isOnboardingVisible = await onboardingHeading.isVisible({ timeout: 5000 }).catch(() => false)
+ const cloudIdeOption = page.getByTestId('onboarding-cloud-ide-option').first()
+ const ideOption = page.getByTestId('onboarding-ide-option').first()
+ const isOnboardingVisible =
+ (await cloudIdeOption.isVisible({ timeout: 5000 }).catch(() => false)) ||
+ (await ideOption.isVisible({ timeout: 5000 }).catch(() => false))
...
- const cloudIdeHeading = page.locator('h3', { hasText: '使用WeCode云IDE' }).first()
- const isCloudIdeVisible = await cloudIdeHeading.isVisible({ timeout: 3000 }).catch(() => false)
+ const isCloudIdeVisible = await cloudIdeOption.isVisible({ timeout: 3000 }).catch(() => false)
...
- await cloudIdeHeading.click({ timeout: 2000 })
+ await cloudIdeOption.click({ timeout: 2000 })
...
- await page.getByText('在IDE中使用WeCode').first().click({ timeout: 2000 })
+ await ideOption.click({ timeout: 2000 })Also applies to: 65-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/specs/code-flow.spec.ts` around lines 58 - 60, The test is locating the
onboarding dialog by localized heading text via onboardingHeading =
page.locator('h2', { hasText: ... }) which is brittle; update the locator to use
the stable data-testid attributes for the onboarding dialog/cards (replace the
page.locator call that sets onboardingHeading and any similar locators in the
65-129 range) so tests target the onboarding test IDs instead of localized copy
or XPath; ensure you update all occurrences (the onboardingHeading variable and
any other page.locator references for onboarding elements) to use the
appropriate data-testid strings provided in the PR.
| let clicked = false | ||
|
|
||
| // Strategy 1: Try clicking the heading itself (it might have click handler) | ||
| try { | ||
| await cloudIdeHeading.click({ timeout: 2000 }) | ||
| console.log('✓ Clicked: 使用WeCode云IDE (heading)') | ||
| clicked = true | ||
| } catch { |
There was a problem hiding this comment.
Fail fast if the onboarding dialog is still open.
Right now this branch sleeps for 3 seconds and logs completion even when every click attempt falls through. Throw when no option was clicked and explicitly wait for the onboarding UI to disappear; otherwise the next clicks can still be intercepted by the modal.
✅ Suggested change
if (isOnboardingVisible) {
console.log('✓ Code feature onboarding page detected, selecting cloud IDE option...')
+ let clicked = false
// Click the "使用WeCode云IDE" (Use WeCode Cloud IDE) option
// Find by the h3 heading and click on its parent card
const cloudIdeHeading = page.locator('h3', { hasText: '使用WeCode云IDE' }).first()
const isCloudIdeVisible = await cloudIdeHeading.isVisible({ timeout: 3000 }).catch(() => false)
if (isCloudIdeVisible) {
// Strategy: Find the clickable parent element of the h3 heading
// Try multiple approaches to find the actual clickable card
- let clicked = false
-
// Strategy 1: Try clicking the heading itself (it might have click handler)
try {
await cloudIdeHeading.click({ timeout: 2000 })
console.log('✓ Clicked: 使用WeCode云IDE (heading)')
clicked = true
@@
} else {
// Fallback: try the first option (在IDE中使用WeCode)
try {
await page.getByText('在IDE中使用WeCode').first().click({ timeout: 2000 })
console.log('✓ Clicked: 在IDE中使用WeCode')
+ clicked = true
} catch {
console.log('✗ Could not click any onboarding option')
}
}
- // Wait for the onboarding page to disappear
- await page.waitForTimeout(3000)
+ if (!clicked) {
+ throw new Error('Could not dismiss the code-feature onboarding')
+ }
+ await expect(onboardingHeading).toBeHidden({ timeout: 10000 })
console.log('✓ Onboarding completed, continuing with test...')
}Also applies to: 123-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/specs/code-flow.spec.ts` around lines 73 - 80, The test currently tries
multiple click strategies on cloudIdeHeading but sleeps on failure; instead,
after the click attempts using cloudIdeHeading and subsequent strategies (the
clicked flag), if clicked is still false throw an error to fail fast and call a
visibility wait to ensure the onboarding modal/dialog is gone (e.g., await
page.waitForSelector('<onboarding-selector>', { state: 'hidden', timeout: 5000
}) or use the existing onboarding dialog locator) before proceeding so
subsequent clicks aren't intercepted; update the block around cloudIdeHeading
and the similar block at lines 123-135 to throw when no option was clicked and
to explicitly wait for the onboarding UI to disappear.
| const repoOptions = page.locator('[data-testid^="repo-option-"]') | ||
| const count = await repoOptions.count() | ||
| console.log(`Found ${count} repo options`) | ||
| if (count > 0) { | ||
| // Find first option containing wegent | ||
| for (let i = 0; i < count; i++) { | ||
| const text = await repoOptions.nth(i).textContent() | ||
| console.log(`Repo option ${i}: ${text}`) | ||
| if (text && text.includes('wegent')) { | ||
| await repoOptions.nth(i).click() | ||
| console.log('Selected repository by text match') | ||
| break | ||
| } |
There was a problem hiding this comment.
Assert the exact repo/branch here, not just “something got selected”.
The fallback picks the first option whose text contains wegent, which is ambiguous when sibling nested repos exist; backend/app/services/git_skill/utils.py:108-113 already shows .../wegent-skills-style paths. Then the final guard only rejects Chinese placeholder text, so the test can keep running against the wrong repo/branch or an English placeholder state.
🧭 Suggested change
- if (text && text.includes('wegent')) {
+ if (text?.trim() === 'weibo_rd/common/wecode/wegent') {
await repoOptions.nth(i).click()
console.log('Selected repository by text match')
break
}
...
- if (!repoBranchText || repoBranchText.includes('请选择') || repoBranchText.includes('选择')) {
- throw new Error('Repository was not properly selected. Selector text: ' + repoBranchText)
+ const normalizedRepoBranchText = repoBranchText?.replace(/\s+/g, ' ').trim() ?? ''
+ if (
+ !normalizedRepoBranchText.includes('weibo_rd/common/wecode/wegent') ||
+ !normalizedRepoBranchText.includes('main')
+ ) {
+ throw new Error(`Repository/branch selection mismatch: ${normalizedRepoBranchText}`)
}Also applies to: 284-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/specs/code-flow.spec.ts` around lines 217 - 229, The test currently
selects the first repo option whose visible text contains 'wegent', which is
ambiguous; change selection to match the exact data-testid value exposed by
RepoListView (the full repo label present in the repo option data-testid)
instead of using text.includes. Locate the repoOptions locator and replace the
fuzzy match loop (using textContent/includes 'wegent') with a check against the
exact expected data-testid (e.g., compare
repoOptions.nth(i).getAttribute('data-testid') to the known full repo label) and
click when equal; then assert the selected repo/branch equals that exact
expected label (also update the analogous logic at lines ~284-287) so the test
fails if a sibling nested repo is chosen.
1 participant
Summary
修复 code-flow E2E 测试中 onboarding page 处理失败的问题。
Problem
测试在 "Bug or feature?" / "选择最适合您的方式继续编码之旅" 引导页卡住，
因为点击逻辑不正确。
Solution
添加了多种点击策略来确保可以关闭引导页：
Testing
should analyze repository and provide code suggestions测试通过should create a new file in the repository测试通过Summary by CodeRabbit
Note: This release contains internal test infrastructure improvements with no user-facing changes.