Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tests): Move leftover acceptance tests for users from drone to Cypress #41021

Merged
merged 1 commit into from
Oct 25, 2023
Merged

fix(tests): Move leftover acceptance tests for users from drone to Cypress #41021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions .drone.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,36 +1621,6 @@ trigger:
- pull_request
- push

---
kind: pipeline
name: acceptance-users

steps:
- name: submodules
image: ghcr.io/nextcloud/continuous-integration-alpine-git:latest
commands:
- git submodule update --init
- name: acceptance-users
image: ghcr.io/nextcloud/continuous-integration-acceptance-php8.0:latest
commands:
- tests/acceptance/run-local.sh --timeout-multiplier 10 --nextcloud-server-domain acceptance-users --selenium-server selenium:4444 allow-git-repository-modifications features/users.feature

services:
- name: selenium
image: ghcr.io/nextcloud/continuous-integration-selenium:3.141.59
environment:
# Reduce default log level for Selenium server (INFO) as it is too
# verbose.
JAVA_OPTS: -Dselenium.LOGGER.level=WARNING

trigger:
branch:
- master
- stable*
event:
- pull_request
- push

---
kind: pipeline
name: acceptance-apps
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/cypress.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:

steps:
- name: Checkout app
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v3.5.2
uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0

- name: Check composer.json
id: check_composer
Expand All @@ -39,8 +39,8 @@ jobs:
uses: skjnldsv/read-package-engines-version-actions@8205673bab74a63eb9b8093402fd9e0e018663a1 # v2.2
id: versions
with:
fallbackNode: "^14"
fallbackNpm: "^7"
fallbackNode: "^20"
fallbackNpm: "^9"

- name: Set up node ${{ steps.versions.outputs.nodeVersion }}
uses: actions/setup-node@5e21ff4d9bc1a8cf6de233a3057d20ec6b3fb69d # v3.8.1
Expand Down
2 changes: 1 addition & 1 deletion apps/settings/src/components/UserList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
:data-component="UserRow"
:data-sources="filteredUsers"
data-key="id"
data-test-id="userList"
data-cy-user-list
:item-height="rowHeight"
:style="style"
:extra-props="{
Expand Down
12 changes: 12 additions & 0 deletions apps/settings/src/components/Users/UserListHeader.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@
<template>
<tr class="header">
<th class="header__cell header__cell--avatar"
data-cy-user-list-header-avatar
scope="col">
<span class="hidden-visually">
{{ t('settings', 'Avatar') }}
</span>
</th>
<th class="header__cell header__cell--displayname"
data-cy-user-list-header-displayname
scope="col">
<strong>
{{ t('settings', 'Display name') }}
Expand All @@ -39,33 +41,40 @@
</th>
<th class="header__cell"
:class="{ 'header__cell--obfuscated': hasObfuscated }"
data-cy-user-list-header-password
scope="col">
<span>{{ passwordLabel }}</span>
</th>
<th class="header__cell"
data-cy-user-list-header-email
scope="col">
<span>{{ t('settings', 'Email') }}</span>
</th>
<th class="header__cell header__cell--large"
data-cy-user-list-header-groups
scope="col">
<span>{{ t('settings', 'Groups') }}</span>
</th>
<th v-if="subAdminsGroups.length > 0 && settings.isAdmin"
class="header__cell header__cell--large"
data-cy-user-list-header-subadmins
scope="col">
<span>{{ t('settings', 'Group admin for') }}</span>
</th>
<th class="header__cell"
data-cy-user-list-header-quota
scope="col">
<span>{{ t('settings', 'Quota') }}</span>
</th>
<th v-if="showConfig.showLanguages"
class="header__cell header__cell--large"
data-cy-user-list-header-languages
scope="col">
<span>{{ t('settings', 'Language') }}</span>
</th>
<th v-if="showConfig.showUserBackend || showConfig.showStoragePath"
class="header__cell header__cell--large"
data-cy-user-list-header-storage-location
scope="col">
<span v-if="showConfig.showUserBackend">
{{ t('settings', 'User backend') }}
Expand All @@ -77,15 +86,18 @@
</th>
<th v-if="showConfig.showLastLogin"
class="header__cell"
data-cy-user-list-header-last-login
scope="col">
<span>{{ t('settings', 'Last login') }}</span>
</th>
<th class="header__cell header__cell--large header__cell--fill"
data-cy-user-list-header-manager
scope="col">
<!-- TRANSLATORS This string describes a manager in the context of an organization -->
<span>{{ t('settings', 'Manager') }}</span>
</th>
<th class="header__cell header__cell--actions"
data-cy-user-list-header-actions
scope="col">
<span class="hidden-visually">
{{ t('settings', 'User actions') }}
Expand Down
50 changes: 31 additions & 19 deletions apps/settings/src/components/Users/UserRow.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@

<template>
<tr class="user-list__row"
:data-test="user.id">
<td class="row__cell row__cell--avatar">
:data-cy-user-row="user.id">
<td class="row__cell row__cell--avatar" data-cy-user-list-cell-avatar>
<NcLoadingIcon v-if="isLoadingUser"
:name="t('settings', 'Loading user …')"
:size="32" />
Expand All @@ -36,12 +36,12 @@
:user="user.id" />
</td>

<td class="row__cell row__cell--displayname" data-test-id="cell-displayname">
<td class="row__cell row__cell--displayname" data-cy-user-list-cell-displayname>
<template v-if="editing && user.backendCapabilities.setDisplayName">
<NcTextField ref="displayNameField"
class="user-row-text-field"
data-test-id="input-displayName"
Copy link
Member

@skjnldsv skjnldsv Oct 22, 2023 •
edited by artonge
Loading

Choose a reason for hiding this comment

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

Ah, we talked abut this before with various other devs. I advocated for the usage of data-cy-xxxx and seemed that other devs agreed too after a some discussion. This allow to scope the elements.

I know what the best-practices says, but I think they do not have our overall ecosystem, so things differs a bit :)

The data attribute value should be used when it have a real changing value. Like data-cy-userslist-user="admin". Otherwise it makes really weird to target a specific element.
I also prefer to have more unique data attributes. The fact that you can grep all [data-cy="files-list-row"] makes no sense imho.
But [data-cy-files-list-row] and [data-cy-files-list-row-fileid="12345"] does :)

Any thoughts or counter arguments?
cc @artonge @max-nextcloud @Pytal as I recall you were in that discussion before 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me it makes no difference if I use data-testid or data-cy both a pretty common and standard, so I am fine with both.

The fact that you can grep all [data-cy="files-list-row"] makes no sense imho.
But [data-cy-files-list-row] and [data-cy-files-list-row-fileid="12345"] does :)

Well for me not that much 😅
data-cy="files-list-row" for me this is like the id (without its singleton limitation), so it says "Select all elements that are file list rows". While data-cy-files-list-row-fileid="12345" says "get the file list row that for file ID 12345".

So my problem with data-cy-files-list-row vs data-cy="files-list-row" is that there is no consistent naming for identifying elements, you always need to use a specific attribute rather that a specific attribute ID.
This feels wrong in a HTML context.

But this is not about feels, so I have no problem to stick with one notation for all of our code and if it was decided to go with data-cy-some-identifier as boolean attribute then I am fine with it. As there is no technical downside 😃

Need some time to adjust this PR then for data-cy and we should probably add this to our development guides :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TL;DR: Will take some time this week to adjust the test selectors 😉

Copy link
Member

Choose a reason for hiding this comment

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

data-cy="files-list-row" for me this is like the id (without its singleton limitation), so it says "Select all elements that are file list rows". While data-cy-files-list-row-fileid="12345" says "get the file list row that for file ID 12345".

If you want all the lines, then you can also do [data-cy-files-list-row-fileid], without the id.
That's the benefit of scoped selectors. And with [data-cy="files-list-row"], you cannot select by Ids with that syntax.
Or with something like [data-cy="files-list-row-123456"], but I find it incredibly messy 🤷

:data-test-loading="`${loading.displayName}`"
data-cy-user-list-input-displayname
:data-loading="loading.displayName || undefined"
:trailing-button-label="t('settings', 'Submit')"
:class="{ 'icon-loading-small': loading.displayName }"
:show-trailing-button="true"
Expand All @@ -63,13 +63,13 @@
</template>
</td>

<td data-test-id="cell-password"
<td data-cy-user-list-cell-password
class="row__cell"
:class="{ 'row__cell--obfuscated': hasObfuscated }">
<template v-if="editing && settings.canChangePassword && user.backendCapabilities.setPassword">
<NcTextField class="user-row-text-field"
data-test-id="input-password"
:data-test-loading="`${loading.password}`"
data-cy-user-list-input-password
:data-loading="loading.password || undefined"
:trailing-button-label="t('settings', 'Submit')"
:class="{'icon-loading-small': loading.password}"
:show-trailing-button="true"
Expand All @@ -91,10 +91,12 @@
</span>
</td>

<td class="row__cell" data-test-id="cell-email">
<td class="row__cell" data-cy-user-list-cell-email>
<template v-if="editing">
<NcTextField class="user-row-text-field"
:class="{'icon-loading-small': loading.mailAddress}"
data-cy-user-list-input-email
:data-loading="loading.mailAddress || undefined"
:show-trailing-button="true"
:trailing-button-label="t('settings', 'Submit')"
:label="t('settings', 'Set new email address')"
Expand All @@ -113,13 +115,15 @@
</span>
</td>

<td class="row__cell row__cell--large row__cell--multiline" data-test-id="cell-groups">
<td class="row__cell row__cell--large row__cell--multiline" data-cy-user-list-cell-groups>
<template v-if="editing">
<label class="hidden-visually"
:for="'groups' + uniqueId">
{{ t('settings', 'Add user to group') }}
</label>
<NcSelect :input-id="'groups' + uniqueId"
<NcSelect data-cy-user-list-input-groups
:data-loading="loading.groups || undefined"
:input-id="'groups' + uniqueId"
:close-on-select="false"
:disabled="isLoadingField"
:loading="loading.groups"
Expand All @@ -143,14 +147,16 @@
</td>

<td v-if="subAdminsGroups.length > 0 && settings.isAdmin"
data-test-id="cell-subadmins"
data-cy-user-list-cell-subadmins
class="row__cell row__cell--large row__cell--multiline">
<template v-if="editing && settings.isAdmin && subAdminsGroups.length > 0">
<label class="hidden-visually"
:for="'subadmins' + uniqueId">
{{ t('settings', 'Set user as admin for') }}
</label>
<NcSelect :input-id="'subadmins' + uniqueId"
<NcSelect data-cy-user-list-input-subadmins
:data-loading="loading.subadmins || undefined"
:input-id="'subadmins' + uniqueId"
:close-on-select="false"
:disabled="isLoadingField"
:loading="loading.subadmins"
Expand All @@ -170,7 +176,7 @@
</span>
</td>

<td class="row__cell" data-test-id="cell-quota">
<td class="row__cell" data-cy-user-list-cell-quota>
<template v-if="editing">
<label class="hidden-visually"
:for="'quota' + uniqueId">
Expand All @@ -179,6 +185,8 @@
<NcSelect v-model="editedUserQuota"
:close-on-select="true"
:create-option="validateQuota"
data-cy-user-list-input-quota
:data-loading="loading.quota || undefined"
:disabled="isLoadingField"
:loading="loading.quota"
:append-to-body="false"
Expand All @@ -202,13 +210,15 @@

<td v-if="showConfig.showLanguages"
class="row__cell row__cell--large"
data-test-id="cell-language">
data-cy-user-list-cell-language>
<template v-if="editing">
<label class="hidden-visually"
:for="'language' + uniqueId">
{{ t('settings', 'Set the language') }}
</label>
<NcSelect :id="'language' + uniqueId"
data-cy-user-list-input-language
:data-loading="loading.languages || undefined"
:allow-empty="false"
:disabled="isLoadingField"
:loading="loading.languages"
Expand All @@ -226,7 +236,7 @@
</td>

<td v-if="showConfig.showUserBackend || showConfig.showStoragePath"
data-test-id="cell-storageLocation"
data-cy-user-list-cell-storage-location
class="row__cell row__cell--large">
<template v-if="!isObfuscated">
<span v-if="showConfig.showUserBackend">{{ user.backend }}</span>
Expand All @@ -241,18 +251,20 @@
<td v-if="showConfig.showLastLogin"
:title="userLastLoginTooltip"
class="row__cell"
data-test-id="cell-lastLogin">
data-cy-user-list-cell-last-login>
<span v-if="!isObfuscated">{{ userLastLogin }}</span>
</td>

<td class="row__cell row__cell--large row__cell--fill" data-test-id="cell-manager">
<td class="row__cell row__cell--large row__cell--fill" data-cy-user-list-cell-manager>
<template v-if="editing">
<label class="hidden-visually"
:for="'manager' + uniqueId">
{{ managerLabel }}
</label>
<NcSelect v-model="currentManager"
class="select--fill"
data-cy-user-list-input-manager
:data-loading="loading.manager || undefined"
:input-id="'manager' + uniqueId"
:close-on-select="true"
:disabled="isLoadingField"
Expand All @@ -271,7 +283,7 @@
</span>
</td>

<td class="row__cell row__cell--actions" data-test-id="cell-actions">
<td class="row__cell row__cell--actions" data-cy-user-list-cell-actions>
<UserRowActions v-if="visible && !isObfuscated && canEdit && !loading.all"
:actions="userActions"
:disabled="isLoadingField"
Expand Down
3 changes: 1 addition & 2 deletions apps/settings/src/components/Users/UserRowActions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
<NcActions :aria-label="t('settings', 'Toggle user actions menu')"
:disabled="disabled"
:inline="1">
<NcActionButton data-test-id="button-toggleEdit"
:data-test="`${edit}`"
<NcActionButton :data-cy-user-list-action-toggle-edit="`${edit}`"
:disabled="disabled"
@click="toggleEdit">
{{ edit ? t('settings', 'Done') : t('settings', 'Edit') }}
Expand Down
Loading
Loading