[Syncing] Small PR to lay foundations for Testing filtering and to sync with upstream#51
Merged
Merged
Conversation
Added more compatibility questions
Added compatibility question tests and verifications Updated tests to cover Keywords and Headline changes recently made Updated tests to cover all of the big5 personality traits
Updated signUp.spec.ts to use new fixture Updated Account information variable names Deleted "deleteUserFixture.ts" as it was incorporated into the "base.ts" file
Updated seedDatabase.ts to throw an error if the user already exists, to also add display names and usernames so they seedUser func acts like a normal basic user Some organising of the google auth code
Added account deletion checks
Updated filters.tsx: added data testid
Added data test attributes to search.tsx and profile-grid.tsx
|
@O-Bots is attempting to deploy a commit to the Compass Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR establishes comprehensive end-to-end testing infrastructure for the People page. It expands test account seeding with personality traits and preferences, introduces Playwright page objects for navigating and filtering the People page, instruments UI components with data-testid attributes for deterministic selection, and adds integration tests that verify filter behavior. ChangesPeople Page E2E Testing Infrastructure
Misc formatting, CI, and small adjustments
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
tests/e2e/web/pages/peoplePage.ts (4)
210-216: 💤 Low valueAddress non-functional search method.
The inline comment indicates this method "Doesn't actually work." Either remove it, fix the implementation, or mark it clearly as a TODO/WIP if it's intended for future work.
Do you want me to investigate why the search isn't working and propose a fix?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/peoplePage.ts` around lines 210 - 216, The useSearch method is currently non-functional; fix it by ensuring the search input (this.searchBox) is focused and the Enter is sent to that element (use this.searchBox.press('Enter') instead of page.keyboard.press), await the action (await this.searchBox.fill(item); await this.searchBox.press('Enter')), and then wait for a reliable post-search signal (e.g., await this.page.waitForResponse(...) matching the search endpoint or await this.page.waitForSelector(...) for a result container). If the search flow is intentionally incomplete, replace the inline comment with a TODO including which part is missing (focus/submit/wait) and mark the method as WIP.
254-260: 💤 Low valueRemove commented-out code.
Lines 256-259 contain commented-out code that duplicates the issue flagged in
setConnectionTypeFilter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/peoplePage.ts` around lines 254 - 260, Remove the commented-out duplicate logic inside the async function setGenderTypeFilter: delete the four commented lines that mimic the interaction (the commented expect/click/getByLabel calls) so the function only calls this.selectOption(this.genderDropdown, genderType[0]); keep behavior consistent with setConnectionTypeFilter and ensure no leftover commented code remains in setGenderTypeFilter, referencing the genderDropdown and GenderTuple usage as the location to edit.
233-239: 💤 Low valueRemove commented-out code.
Lines 235-238 contain commented-out code that should be removed to keep the codebase clean.
♻️ Cleanup
async setConnectionTypeFilter(connectionType: ConnectionTypeTuple) { await this.selectOption(this.connectionTypeDropdown, connectionType[0]) - // await expect(this.connectionTypeDropdown).toBeVisible() - // await this.connectionTypeDropdown.click() - // await expect(this.page.getByLabel(connectionType[0])).toBeVisible() - // await this.page.getByLabel(connectionType[0]).click() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/peoplePage.ts` around lines 233 - 239, The setConnectionTypeFilter function contains four commented-out lines that should be removed to clean up the code; open the setConnectionTypeFilter method and delete the commented-out calls (the lines calling toBeVisible, click, and getByLabel) so only the active await this.selectOption(this.connectionTypeDropdown, connectionType[0]) remains, leaving function behavior unchanged and eliminating dead code.
379-401: ⚡ Quick winSimplify display filter checkbox logic.
The conditional logic in lines 392-398 contains redundant checks. The code can be simplified for better readability.
♻️ Simplified logic
async setDisplayFilter(display: DisplayFilter) { await expect(this.displayDropdown).toBeVisible() await this.displayDropdown.click() if (display.cardSize) await this.page.getByRole('button', {name: `${display.cardSize}`}).click() - if (!display.filters) return - if (display.filters?.length > 0) { + if (display.filters && display.filters.length > 0) { for (let i = 0; i < display.filters.length; i++) { const filter = await this.page.getByRole('checkbox', {name: `${display.filters[i][0]}`}) await expect(filter).toBeVisible() const isChecked = await filter.isChecked() + const shouldBeChecked = display.filters[i][1] - if (display.filters[i][1]) { - if (isChecked) continue - if (!isChecked) await filter.click() - } else if (!display.filters[i][1]) { - if (isChecked) await filter.click() - if (!isChecked) continue - } + if (isChecked !== shouldBeChecked) { + await filter.click() + } } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/peoplePage.ts` around lines 379 - 401, The setDisplayFilter method has redundant conditional branches when toggling checkboxes; simplify by deriving the desired boolean (e.g., const desired = display.filters[i][1]) and comparing it once to the current state (const isChecked = await filter.isChecked()) and only call filter.click() if desired !== isChecked, removing the nested if/continue branches; keep the existing visibility checks (this.displayDropdown, filter) and the cardSize click logic unchanged so behavior and element lookup (page.getByRole) remain the same.tests/e2e/web/pages/notificationsPage.ts (1)
3-11: ⚡ Quick winConsider adding action methods.
The
NotificationPageclass defines locators but exposes no action methods. If this is a placeholder for future work, consider adding a TODO comment. Otherwise, add methods likeclickNotificationTab()andclickSettingsTab()that follow the pattern of asserting visibility before clicking.Example action methods
constructor(public readonly page: Page) { this.notificationTab = page.getByTestId('notifications-tab') this.settingsTab = page.getByTestId('settings-tab') } + + async clickNotificationTab() { + await expect(this.notificationTab).toBeVisible() + await this.notificationTab.click() + } + + async clickSettingsTab() { + await expect(this.settingsTab).toBeVisible() + await this.settingsTab.click() + } }As per coding guidelines, "Action methods in page objects must assert
expect(locator).toBeVisible()before interacting."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/notificationsPage.ts` around lines 3 - 11, The NotificationPage currently only defines locators (notificationTab, settingsTab) and must either include a TODO noting it's intentionally empty or implement action methods; add methods on class NotificationPage such as clickNotificationTab() and clickSettingsTab() that first assert visibility with expect(this.notificationTab).toBeVisible() / expect(this.settingsTab).toBeVisible() and then perform the click using the locator, following the project pattern for page-object actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/utils/seedDatabase.ts`:
- Around line 42-47: The seed sets romanticStyle to null when relationshipStyle
!== 'relationship', which leads to pref_romantic_styles being seeded as [null];
change the assignment so that romanticStyle is an empty array or undefined
(e.g., set romanticStyle = [] or omit the field) and ensure the code that writes
userInfo.pref_romantic_styles uses an empty list/omits the key instead of
storing [null]; update the same pattern around the other occurrence (the block
referenced at line ~72) so neither romanticStyle nor pref_romantic_styles ever
become [null].
In `@tests/e2e/web/pages/peoplePage.ts`:
- Around line 416-421: In verifyNumberOfMatchingProfiles, the assertion is
inverted: swap the operands so the actual value parsed from this.profileCount
(parseInt(test)) is asserted against the expected parameter count; update the
expect call in verifyNumberOfMatchingProfiles to use
expect(actual).toStrictEqual(expected) (keep the existing null guard for test
and continue to parse the UI text before asserting).
- Around line 199-204: The selectOption method is calling isVisible() without
awaiting or asserting its result before clicking the option; update it to assert
visibility using await expect(this.page.getByLabel(label, { exact: true
})).toBeVisible() prior to clicking so the page-object follows the guideline of
asserting locator visibility before interaction (keep the existing
expect(trigger).toBeVisible() and trigger.click() as-is, but replace the
isVisible() line with the explicit expect(...).toBeVisible() assertion
referencing the getByLabel locator).
- Around line 155-197: The sliderHelper function contains potential infinite
loops when adjusting sliders (see minSlider/maxSlider,
currentMinValue/currentMaxValue, changedMinValue/changedMaxValue); fix by adding
a guarded iteration limit (e.g., maxAttempts) to both while loops and also check
for NaN or unchanged value between iterations to break early if aria-valuenow is
null/NaN or stops changing; ensure you still click the correct slider before
attempts and use the existing keyboard presses, but bail out (break) when
attempts exceed the limit or the numeric value does not change between loop
iterations.
In `@tests/e2e/web/specs/signIn.spec.ts`:
- Around line 20-27: The test currently returns early when totalProfiles or
filterdProfiles is falsy, allowing a silent pass; change this to hard assertions
instead: assert that both totalProfiles and filterdProfiles are
defined/non-empty (e.g., using expect(totalProfiles).toBeTruthy() and
expect(filterdProfiles).toBeTruthy()) before parsing and comparing counts, so
that parseInt(totalProfiles) and parseInt(filterdProfiles) are only executed
when valid; update the block around totalProfiles, filterdProfiles,
app.people.profileCountLocator and app.people.setConnectionTypeFilter
accordingly to fail the test loudly if the locator text is missing.
In `@web/components/layout/tabs.tsx`:
- Around line 142-150: The rendered tab lines use tab.title.split('\n').map(...)
creating multiple <Row> elements that all share the same data-testid, causing
duplicate test IDs; update the code in tabs.tsx where tab.title is mapped by
either (a) move the data-testid={`${tab.title.toLowerCase()}-tab`} to the parent
container that wraps the mapped <Row> elements so the whole tab retains a single
test id, or (b) make each mapped <Row> test id unique by appending the index
(e.g. data-testid={`${tab.title.toLowerCase()}-tab-${i}`}); locate the mapping
that uses tab.title.split('\n').map and change the data-testid placement or
value accordingly.
---
Nitpick comments:
In `@tests/e2e/web/pages/notificationsPage.ts`:
- Around line 3-11: The NotificationPage currently only defines locators
(notificationTab, settingsTab) and must either include a TODO noting it's
intentionally empty or implement action methods; add methods on class
NotificationPage such as clickNotificationTab() and clickSettingsTab() that
first assert visibility with expect(this.notificationTab).toBeVisible() /
expect(this.settingsTab).toBeVisible() and then perform the click using the
locator, following the project pattern for page-object actions.
In `@tests/e2e/web/pages/peoplePage.ts`:
- Around line 210-216: The useSearch method is currently non-functional; fix it
by ensuring the search input (this.searchBox) is focused and the Enter is sent
to that element (use this.searchBox.press('Enter') instead of
page.keyboard.press), await the action (await this.searchBox.fill(item); await
this.searchBox.press('Enter')), and then wait for a reliable post-search signal
(e.g., await this.page.waitForResponse(...) matching the search endpoint or
await this.page.waitForSelector(...) for a result container). If the search flow
is intentionally incomplete, replace the inline comment with a TODO including
which part is missing (focus/submit/wait) and mark the method as WIP.
- Around line 254-260: Remove the commented-out duplicate logic inside the async
function setGenderTypeFilter: delete the four commented lines that mimic the
interaction (the commented expect/click/getByLabel calls) so the function only
calls this.selectOption(this.genderDropdown, genderType[0]); keep behavior
consistent with setConnectionTypeFilter and ensure no leftover commented code
remains in setGenderTypeFilter, referencing the genderDropdown and GenderTuple
usage as the location to edit.
- Around line 233-239: The setConnectionTypeFilter function contains four
commented-out lines that should be removed to clean up the code; open the
setConnectionTypeFilter method and delete the commented-out calls (the lines
calling toBeVisible, click, and getByLabel) so only the active await
this.selectOption(this.connectionTypeDropdown, connectionType[0]) remains,
leaving function behavior unchanged and eliminating dead code.
- Around line 379-401: The setDisplayFilter method has redundant conditional
branches when toggling checkboxes; simplify by deriving the desired boolean
(e.g., const desired = display.filters[i][1]) and comparing it once to the
current state (const isChecked = await filter.isChecked()) and only call
filter.click() if desired !== isChecked, removing the nested if/continue
branches; keep the existing visibility checks (this.displayDropdown, filter) and
the cardSize click logic unchanged so behavior and element lookup
(page.getByRole) remain the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2277a862-323b-48f9-9743-17eb81db0b50
📒 Files selected for processing (19)
.github/workflows/ci.ymlbackend/email/emails/new-endorsement.tsxbackend/email/emails/new-message.tsxcommon/src/choices.tstests/e2e/backend/utils/userInformation.tstests/e2e/utils/firebaseUtils.tstests/e2e/utils/seedDatabase.tstests/e2e/web/pages/app.tstests/e2e/web/pages/authPage.tstests/e2e/web/pages/notificationsPage.tstests/e2e/web/pages/peoplePage.tstests/e2e/web/specs/signIn.spec.tstests/e2e/web/utils/accountInformation.tsweb/components/filters/big5-filter.tsxweb/components/filters/filters.tsxweb/components/filters/search.tsxweb/components/layout/tabs.tsxweb/components/page-base.tsxweb/components/profile-grid.tsx
Comment on lines
+20
to
+27
| const totalProfiles = await app.people.profileCountLocator.textContent() | ||
| await app.people.setConnectionTypeFilter(['Collaboration', 'collaboration']) | ||
|
|
||
| const filterdProfiles = await app.people.profileCountLocator.textContent() | ||
|
|
||
| if (!totalProfiles || !filterdProfiles) return | ||
| await expect(parseInt(totalProfiles)).not.toEqual(parseInt(filterdProfiles)) | ||
| }) |
There was a problem hiding this comment.
Avoid silent pass when count text is missing.
Line 25 returns early, so this test can pass without validating behavior. Convert it to hard assertions before comparing counts.
Suggested fix
- const totalProfiles = await app.people.profileCountLocator.textContent()
+ const totalProfiles = await app.people.profileCountLocator.textContent()
await app.people.setConnectionTypeFilter(['Collaboration', 'collaboration'])
- const filterdProfiles = await app.people.profileCountLocator.textContent()
+ const filterdProfiles = await app.people.profileCountLocator.textContent()
- if (!totalProfiles || !filterdProfiles) return
- await expect(parseInt(totalProfiles)).not.toEqual(parseInt(filterdProfiles))
+ expect(totalProfiles).toBeTruthy()
+ expect(filterdProfiles).toBeTruthy()
+
+ const totalCount = Number.parseInt(totalProfiles!, 10)
+ const filteredCount = Number.parseInt(filterdProfiles!, 10)
+ expect(Number.isNaN(totalCount)).toBe(false)
+ expect(Number.isNaN(filteredCount)).toBe(false)
+ await expect(filteredCount).not.toEqual(totalCount)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const totalProfiles = await app.people.profileCountLocator.textContent() | |
| await app.people.setConnectionTypeFilter(['Collaboration', 'collaboration']) | |
| const filterdProfiles = await app.people.profileCountLocator.textContent() | |
| if (!totalProfiles || !filterdProfiles) return | |
| await expect(parseInt(totalProfiles)).not.toEqual(parseInt(filterdProfiles)) | |
| }) | |
| const totalProfiles = await app.people.profileCountLocator.textContent() | |
| await app.people.setConnectionTypeFilter(['Collaboration', 'collaboration']) | |
| const filterdProfiles = await app.people.profileCountLocator.textContent() | |
| expect(totalProfiles).toBeTruthy() | |
| expect(filterdProfiles).toBeTruthy() | |
| const totalCount = Number.parseInt(totalProfiles!, 10) | |
| const filteredCount = Number.parseInt(filterdProfiles!, 10) | |
| expect(Number.isNaN(totalCount)).toBe(false) | |
| expect(Number.isNaN(filteredCount)).toBe(false) | |
| await expect(filteredCount).not.toEqual(totalCount) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/web/specs/signIn.spec.ts` around lines 20 - 27, The test currently
returns early when totalProfiles or filterdProfiles is falsy, allowing a silent
pass; change this to hard assertions instead: assert that both totalProfiles and
filterdProfiles are defined/non-empty (e.g., using
expect(totalProfiles).toBeTruthy() and expect(filterdProfiles).toBeTruthy())
before parsing and comparing counts, so that parseInt(totalProfiles) and
parseInt(filterdProfiles) are only executed when valid; update the block around
totalProfiles, filterdProfiles, app.people.profileCountLocator and
app.people.setConnectionTypeFilter accordingly to fail the test loudly if the
locator text is missing.
Comment on lines
142
to
150
| {tab.title.split('\n').map((line, i) => ( | ||
| <Row className={'items-center justify-center'} key={i}> | ||
| <Row | ||
| className={'items-center justify-center'} | ||
| key={i} | ||
| data-testid={`${tab.title.toLowerCase()}-tab`} | ||
| > | ||
| {line} | ||
| </Row> | ||
| ))} |
There was a problem hiding this comment.
Duplicate test IDs when tab title contains newlines.
When tab.title contains newlines, the .split('\n').map() creates multiple <Row> elements, each with the identical data-testid value (line 146). HTML requires unique data-testid values within a document. This will cause test selectors to fail or select the wrong element.
🐛 Proposed fix with unique test IDs
<Col>
{tab.title.split('\n').map((line, i) => (
<Row
className={'items-center justify-center'}
key={i}
- data-testid={`${tab.title.toLowerCase()}-tab`}
+ data-testid={`${tab.title.toLowerCase()}-tab${i > 0 ? `-line-${i}` : ''}`}
>
{line}
</Row>
))}
</Col>Alternatively, if tests only need to identify the tab (not individual lines), move the data-testid to the parent container:
- <Col
- className={
- clsx()
- // tab.stackedTabIcon && (activeIndex !== i) && 'opacity-85'
- }
- >
+ <Col
+ className={clsx()}
+ data-testid={`${tab.title.toLowerCase()}-tab`}
+ >
<Tooltip text={tab.tooltip}>
{tab.stackedTabIcon && <Row className="justify-center">{tab.stackedTabIcon}</Row>}
<Row className={'items-center'}>
<Col>
{tab.title.split('\n').map((line, i) => (
- <Row
- className={'items-center justify-center'}
- key={i}
- data-testid={`${tab.title.toLowerCase()}-tab`}
- >
+ <Row className={'items-center justify-center'} key={i}>
{line}
</Row>
))}
</Col>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {tab.title.split('\n').map((line, i) => ( | |
| <Row className={'items-center justify-center'} key={i}> | |
| <Row | |
| className={'items-center justify-center'} | |
| key={i} | |
| data-testid={`${tab.title.toLowerCase()}-tab`} | |
| > | |
| {line} | |
| </Row> | |
| ))} | |
| {tab.title.split('\n').map((line, i) => ( | |
| <Row | |
| className={'items-center justify-center'} | |
| key={i} | |
| data-testid={`${tab.title.toLowerCase()}-tab${i > 0 ? `-line-${i}` : ''}`} | |
| > | |
| {line} | |
| </Row> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/components/layout/tabs.tsx` around lines 142 - 150, The rendered tab
lines use tab.title.split('\n').map(...) creating multiple <Row> elements that
all share the same data-testid, causing duplicate test IDs; update the code in
tabs.tsx where tab.title is mapped by either (a) move the
data-testid={`${tab.title.toLowerCase()}-tab`} to the parent container that
wraps the mapped <Row> elements so the whole tab retains a single test id, or
(b) make each mapped <Row> test id unique by appending the index (e.g.
data-testid={`${tab.title.toLowerCase()}-tab-${i}`}); locate the mapping that
uses tab.title.split('\n').map and change the data-testid placement or value
accordingly.
| private readonly profileName: Locator | ||
| private readonly profileAgeGender: Locator | ||
|
|
||
| constructor(public readonly page: Page) { |
Member
There was a problem hiding this comment.
I like that you get by role whenever possible and only use get by test ID when unavoidable!
Co-authored-by: Martin Braquet <martin.braquet@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/e2e/web/pages/peoplePage.ts (1)
428-432:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUse the parsed UI count and
counthere, not placeholder identifiers.
actualandexpectedare not defined in this scope, so this method either fails type-checking or throws as soon as it runs. Compare the parsed text content directly tocountinstead.🐛 Proposed fix
async verifyNumberOfMatchingProfiles(count: number) { await expect(this.profileCount).toBeVisible() const test = await this.profileCount.textContent() if (!test) return - expect(actual).toStrictEqual(expected) + const actual = Number.parseInt(test, 10) + expect(actual).toStrictEqual(count) }#!/bin/bash # Show the broken assertion and confirm there are no local declarations # for `actual` / `expected` in this file. sed -n '428,433p' tests/e2e/web/pages/peoplePage.ts rg -n --type=ts '\b(const|let|var)\s+(actual|expected)\b' tests/e2e/web/pages/peoplePage.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/web/pages/peoplePage.ts` around lines 428 - 432, The test uses undefined placeholders `actual` and `expected`; update verifyNumberOfMatchingProfiles to read the text from this.profileCount, parse it to a number (e.g., parseInt or Number), handle the empty/null case (throw or fail the test if text is missing), and assert that the parsed numeric value strictly equals the method parameter `count`. Locate the method verifyNumberOfMatchingProfiles and replace the placeholder assertion with a comparison like expect(parsedCount).toStrictEqual(count) after parsing the text content from this.profileCount.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/e2e/web/pages/peoplePage.ts`:
- Around line 428-432: The test uses undefined placeholders `actual` and
`expected`; update verifyNumberOfMatchingProfiles to read the text from
this.profileCount, parse it to a number (e.g., parseInt or Number), handle the
empty/null case (throw or fail the test if text is missing), and assert that the
parsed numeric value strictly equals the method parameter `count`. Locate the
method verifyNumberOfMatchingProfiles and replace the placeholder assertion with
a comparison like expect(parsedCount).toStrictEqual(count) after parsing the
text content from this.profileCount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 157548dd-b47a-4a89-b09b-dd25237c66d0
📒 Files selected for processing (3)
.github/workflows/cd-api.ymltests/e2e/utils/seedDatabase.tstests/e2e/web/pages/peoplePage.ts
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/cd-api.yml
Member
|
Looks great, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary by CodeRabbit
Tests
Chores