test: assert recruiting locators are visible instead of defined#10922
Open
voidmatcha wants to merge 1 commit into
Open
test: assert recruiting locators are visible instead of defined#10922voidmatcha wants to merge 1 commit into
voidmatcha wants to merge 1 commit into
Conversation
The navigator flow checked vacancy and applicant text with expect(locator).toBeDefined(). A Playwright Locator is always defined, so those assertions passed whether or not the text rendered. navigateToVacanciesAndCheckSoftwareEngineer had no other assertion, so a missing "Software Engineer" vacancy or "APP-1" applicant would still pass. Switch the three checks to web-first toBeVisible() so they wait for and verify the text the method name promises to check.
|
Connected to Huly®: UBERF-16525 |
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
1 participant
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.
What
tests/sanity/tests/model/recruiting/recruiting-page.tsasserted recruiting text withexpect(locator).toBeDefined()in two page-object methods. A PlaywrightLocatoris always defined, so all three checks passed regardless of what the page rendered. This switches them to web-firsttoBeVisible(), which auto-waits for and verifies the element each method name claims to check.Why
navigateToVacanciesAndCheckSoftwareEngineerhad no other assertion, so its twotoBeDefined()calls were the method's entire verification. Thenavigatorsanity test intests/sanity/tests/workbench.spec.tscalls it, so a missing "Software Engineer" vacancy or "APP-1" applicant would still pass green. AtoBeDefined()on a Locator can never fail, so the check was dead weight.checkApplicationsVisibilityalready had a realawait expect(...).toBeVisible()above itstoBeDefined()line, so only the always-true line changed there.Tests
I could not run the sanity Playwright suite locally: it needs GitHub Packages auth and a deployed Huly stack. Disclosing that rather than claiming a green run. The change is a three-line assertion swap with no new imports, selectors, or test-title changes, and
git diff --checkis clean.Out of scope
No other assertions or methods were touched. The selectors are unchanged; only the matcher and the missing
awaitwere corrected.Found while reviewing the test suite with
e2e-skills/e2e-reviewer.