Skip to content

[full-ci][tests-only] test: standardize playwright step defnitions#13625

Merged
anon-pradip merged 2 commits intomasterfrom
test/standardize-step-defns
Mar 16, 2026
Merged

[full-ci][tests-only] test: standardize playwright step defnitions#13625
anon-pradip merged 2 commits intomasterfrom
test/standardize-step-defns

Conversation

@anon-pradip
Copy link
Copy Markdown
Contributor

@anon-pradip anon-pradip commented Mar 10, 2026 •
edited
Loading

Description

This PR standardizes Playwright E2E step definitions / naming across the test suite to make steps more consistent and readable.

Main changes:

  • Rename UI step helpers to a consistent “user…” style, e.g.
    • ui.logInUser → ui.userLogsIn
    • ui.logOutUser → ui.userLogsOut
    • ui.navigateToSharedWithMePage → ui.userNavigatesToSharedWithMePage
    • ui.openResourceInViewer → ui.userOpensResourceInViewer
    • ui.uploadResource → ui.userUploadsResources
    • ui.showHiddenFiles → ui.userShowsHiddenFiles
  • Rename API step helpers to a consistent “usersHave…” / “userHas…” style, e.g.
    • api.usersHasBeenCreated → api.usersHaveBeenCreated
    • api.userHasAssignRolesToUsers → api.userHasAssignedRolesToUsers
    • api.createFilesInsideSpaceBySpaceName → api.userHasCreatedFilesInsideSpace
    • api.addUserToGroup → api.usersHaveBeenAddedToGroup
  • Replace boolean-returning “check/exists” helpers + inline expect(...) with explicit assertion-style steps, e.g.
    • ui.checkGroupsPresenceById + expect(...).toBeTruthy()/toBeFalsy() → ui.userShouldSeeGroupIds(...)
    • ui.groupDisplayNameExists + expect(...).toBeTruthy() → ui.userShouldSeeGroupDisplayName(...)
    • ui.isAbleToEditFileOrFolder + expect(...).toBeFalsy() → ui.userShouldNotBeAbleToEditResource(...)

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

Open tasks:

  • ...

@anon-pradip anon-pradip self-assigned this Mar 10, 2026
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch 4 times, most recently from 29bd6ed to 6efa807 Compare March 11, 2026 11:08
@anon-pradip anon-pradip marked this pull request as ready for review March 11, 2026 11:08
@anon-pradip anon-pradip marked this pull request as draft March 11, 2026 11:08
@anon-pradip anon-pradip marked this pull request as ready for review March 11, 2026 11:25
nabim777
nabim777 reviewed Mar 12, 2026
View reviewed changes
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from 6efa807 to e3b935d Compare March 12, 2026 04:21
nabim777
nabim777 reviewed Mar 12, 2026
View reviewed changes
tests/e2e-playwright/specs/file-action/passwordProtectedFolder.spec.ts Outdated

// And "Alice" enables the option to display the hidden file
await ui.showHiddenFiles({ actorsEnvironment, stepUser: 'Alice' })
await ui.userShowsHiddenFiles({ actorsEnvironment, stepUser: 'Alice' })
Copy link
Copy Markdown
Member

@nabim777 nabim777 Mar 12, 2026

Choose a reason for hiding this comment

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

Suggested change
await ui.userShowsHiddenFiles({ actorsEnvironment, stepUser: 'Alice' })
await ui.userEnablesShowHiddenFiles({ actorsEnvironment, stepUser: 'Alice' })

I think it will be suitable

Copy link
Copy Markdown
Contributor Author

@anon-pradip anon-pradip Mar 12, 2026

Choose a reason for hiding this comment

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

🤔 If so then userEnablesShowHiddenFilesOption would be more better? @nabim777

Copy link
Copy Markdown
Member

@nabim777 nabim777 Mar 12, 2026

Choose a reason for hiding this comment

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

Yes, it also looks better.

@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from e3b935d to 2b7ccf2 Compare March 12, 2026 04:24
@anon-pradip anon-pradip requested a review from nabim777 March 12, 2026 04:26
PrajwolAmatya
PrajwolAmatya reviewed Mar 12, 2026
View reviewed changes
tests/e2e-playwright/specs/admin-settings/groups.spec.ts Outdated
Comment on lines +62 to +67
await ui.userShouldSeeGroupIds({
actorsEnvironment,
stepUser: 'Admin',
expectedGroupIds: ['sales'],
shouldBePresent: false
})
Copy link
Copy Markdown
Contributor

@PrajwolAmatya PrajwolAmatya Mar 12, 2026

Choose a reason for hiding this comment

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

The function says user should see group ids, but it is expected that the user should not see. This seems kind of confusing. Maybe have different functions for assertion cases, userShouldSeeGroupIds and userShouldNotSeeGroupIds?
Let's see what others have to say on this.

Copy link
Copy Markdown
Contributor Author

@anon-pradip anon-pradip Mar 12, 2026

Choose a reason for hiding this comment

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

made two separate functions userShouldSeeGroupIds and userShouldNotSeeGroupIds

@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from 2b7ccf2 to 1aa11f5 Compare March 12, 2026 05:03
nabim777
nabim777 reviewed Mar 12, 2026
View reviewed changes
nabim777
nabim777 reviewed Mar 12, 2026
View reviewed changes
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch 2 times, most recently from da2348d to f80ef8c Compare March 12, 2026 08:21
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from f80ef8c to a51e8b6 Compare March 12, 2026 09:02
@anon-pradip anon-pradip requested a review from nabim777 March 12, 2026 09:15
@nabim777
Copy link
Copy Markdown
Member

nabim777 commented Mar 13, 2026 •
edited
Loading

@anon-pradip, could you refactor this method getNotificationMessagestoo?

https://github.com/owncloud/web/blob/master/tests/e2e-playwright/steps/ui/application.ts#L19C7-L19C8

@nabim777
Copy link
Copy Markdown
Member

nabim777 commented Mar 13, 2026 •
edited
Loading

I think method userMarksNotificationsAsRead also need to be renamed as userMarksAllNotificationsAsRead

web/tests/e2e-playwright/steps/ui/account.ts

Line 55 in a51e8b6

export async function userMarksNotificationsAsRead({

nabim777
nabim777 reviewed Mar 13, 2026
View reviewed changes
tests/e2e-playwright/specs/spaces/pagination.spec.ts Outdated

// When "Alice" enables flat list
await ui.toggleFlatList({ actorsEnvironment, stepUser: 'Alice' })
await ui.userTogglesFlatList({ actorsEnvironment, stepUser: 'Alice' })
Copy link
Copy Markdown
Member

@nabim777 nabim777 Mar 13, 2026

Choose a reason for hiding this comment

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

Suggested change
await ui.userTogglesFlatList({ actorsEnvironment, stepUser: 'Alice' })
await ui.userEnablesFlatList({ actorsEnvironment, stepUser: 'Alice' })

nabim777
nabim777 reviewed Mar 13, 2026
View reviewed changes
tests/e2e-playwright/specs/navigation/personalSpacePagination.spec.ts Outdated
await ui.changeItemsPerPage({ actorsEnvironment, stepUser: 'Alice', itemsPerPage: '20' })
await ui.userChangesItemsPerPage({ actorsEnvironment, stepUser: 'Alice', itemsPerPage: '20' })
// Then "Alice" should see the text "26 items with 223 B in total (11 files including 1 hidden, 15 folders)" at the footer of the page
await ui.expectFooterTextToBe({
Copy link
Copy Markdown
Member

@nabim777 nabim777 Mar 13, 2026

Choose a reason for hiding this comment

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

Suggested change
await ui.expectFooterTextToBe({
await ui.userShouldSeeFooterText({

@nabim777
Copy link
Copy Markdown
Member

nabim777 commented Mar 13, 2026 •
edited
Loading

I think these method also needs to be refactored and renamed.

  1. web/tests/e2e-playwright/steps/ui/resources.ts

    Line 284 in a51e8b6

    export async function getCurrentPageNumber({

  2. web/tests/e2e-playwright/steps/ui/resources.ts

    Line 336 in a51e8b6

    export async function getFilesList({

@nabim777
Copy link
Copy Markdown
Member

nabim777 commented Mar 13, 2026 •
edited
Loading

please rename these also

  1. web/tests/e2e-playwright/steps/ui/resources.ts

    Line 659 in a51e8b6

    export async function resourceShouldBeLocked({

  2. web/tests/e2e-playwright/steps/ui/resources.ts

    Line 674 in a51e8b6

    export async function resourceShouldNotBeLocked({

@anon-pradip
Copy link
Copy Markdown
Contributor Author

anon-pradip commented Mar 13, 2026

@nabim777 removed all expects from step defn 🚀

@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from 3c579e4 to cea0845 Compare March 13, 2026 09:14
@anon-pradip anon-pradip requested a review from nabim777 March 13, 2026 09:15
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from cea0845 to 2956a71 Compare March 13, 2026 10:03
@anon-pradip anon-pradip force-pushed the test/standardize-step-defns branch from 2956a71 to c12f2d7 Compare March 13, 2026 10:55
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 13, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nabim777
nabim777 approved these changes Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@nabim777 nabim777 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@anon-pradip anon-pradip merged commit ff9b1ba into master Mar 16, 2026
4 checks passed
@anon-pradip anon-pradip deleted the test/standardize-step-defns branch March 16, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nabim777 nabim777 nabim777 approved these changes

@nirajacharya2 nirajacharya2 Awaiting requested review from nirajacharya2

@saw-jan saw-jan Awaiting requested review from saw-jan

@Salipa-Gurung Salipa-Gurung Awaiting requested review from Salipa-Gurung

@Ashim-Stha Ashim-Stha Awaiting requested review from Ashim-Stha

@PrajwolAmatya PrajwolAmatya Awaiting requested review from PrajwolAmatya

Assignees

@anon-pradip anon-pradip

Labels

QA:team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[QA] Standardize Naming Conventions for Step Definition Functions in E2E Test Specs

3 participants

@anon-pradip @nabim777 @PrajwolAmatya