nextcloud / standards

1 stars 0 forks source link

Cypress selectors library #4

Closed skjnldsv closed 1 year ago

skjnldsv commented 2 years ago

We had the discussion at various moments over the last years and quite frequently over the last months in the server team.

Context

As components/e2e/visual testing gets more and more powerful and practical, we wand to move away from selenium for a much easier testing process like cypress.

Goals

Offer an easy way to select various elements of Nextcloud without having to manually write down all selectors and generating duplicates across the Nextcloud apps/repos

Suggestion

Create a versioned library @nextcloud/cypress where devs can specify selectors for specific apps. Selectors would then be importable in your tests files.

import { UploadButton } from `@nextcloud/cypress/files/24`
import { StartCallButton } from `@nextcloud/cypress/spreed/14`

cy.getNc(UploadButton).should('exist')
cy.getNc(StartCallButton).click()

Any opinions? Questions?

cc @nextcloud/server @juliushaertl @nickvergessen @danxuliu

juliushaertl commented 2 years ago

Definitely, we already talked about that in @nextcloud/office but more in the context of helper methods than selectors, so those could be additionally there I guess:

Possible sharable ones that we have at text might be:

skjnldsv commented 2 years ago

We should also provide commands, definitely! Viewer also does it.

Should the commands also be versionned? How would that looks like? They are not technically registered the same way, so it might be a tad more tricky.

Aside from that, any comments regarding the technical implementation of the selectors ?

max-nextcloud commented 2 years ago

I'd imagine commands to be imported in cypress/support/e2e.js (I believe that used to be index.js).

So something like:

import '@nextcloud/cypress/dist/commands.js'
import './commands.js'

I think versioning and having a matching cypress version as a peer dependency would be good.

juliushaertl commented 2 years ago

Aside from that, any comments regarding the technical implementation of the selectors ?

The proposal seems fine to me đź‘Ť

Maybe for the internal implementation we should follow the cypress best practices to use a data attribute instead of css selectors (which is something that we learned on text the hard way) https://docs.cypress.io/guides/references/best-practices#Selecting-Elements - Mostly up to the implementing apps but we still may state that for reference.

skjnldsv commented 2 years ago

I think versioning and having a matching cypress version as a peer dependency would be good.

Commands versioning could cause issues though. If server changes a process, how do you ensure the imported commands are still valid ? There would only be latest?

come-nc commented 2 years ago

I think versioning and having a matching cypress version as a peer dependency would be good.

Commands versioning could cause issues though. If server changes a process, how do you ensure the imported commands are still valid ? There would only be latest?

You’d have to depend on matching versions for server and cypress library, seems fair. This is how it works for the OCP stubs that @ChristophWurst hosts for psalm.

mejo- commented 2 years ago

Thanks for the initiative, that's a great idea!

For the record, Collectives also uses Cypress for e2e testing (no Component testing yet) and we have a few commands: https://gitlab.com/collectivecloud/collectives/-/blob/main/cypress/support/commands.js

In particular, toggleApp (in order to en-/disable an app) and seedCircle as well as seedCircleMember might be interesting to put into this library.

skjnldsv commented 2 years ago

I think versioning and having a matching cypress version as a peer dependency would be good.

Commands versioning could cause issues though. If server changes a process, how do you ensure the imported commands are still valid ? There would only be latest?

You’d have to depend on matching versions for server and cypress library, seems fair. This is how it works for the OCP stubs that @ChristophWurst hosts for psalm.

so keeping branches for each version? That's not possible. Some apps are not bound to server versions, so you need to be able to feth npm and get access to all selectors versions I'd say

nickvergessen commented 2 years ago

Forwarding my ping to @marcoambrosini and @danxuliu in case they have input. I'm way to backend dev to have an opinion on this level

artonge commented 2 years ago

If I understand correctly, when the version of the tested server change, the dev will sometime need to update the imports ?

Another way would be to do something like:

import SelectorFactory from '@nextcloud/cypress'

const sf = new SelectorFactory(serverVersion) // Only one place where we need to change it.

const UploadButton = sf.get('files', 'UploadButton')
const StartCallButton = sf.get('spreed', 'StartCallButton')

But I don't have any strong opinion against the initial proposal, just wanted to highlight one caveat and present another solution.

Another solution would be to dynamically discover the server and apps version. This would allow us to keep the initial proposal, but without the version. But not sure if this is possible.

// @nextcloud/cypress/files/index.js
const serverVersion = magicalWayOfGuessingTheServerVersion()
export const UploadButton = uploadButtonSelectors[serverVersion]

// my_test.js
import { UploadButton } from `@nextcloud/cypress/files`
vinicius73 commented 2 years ago

Version can be passed as parameter in cypress plugin config. The project, who uses this plugin, just can use environment variables to define it.

skjnldsv commented 2 years ago

Okay, a bit more thoughts, and trying to think differently here. @come-nc had the idea of using fallback, so if your application vhanges selector, you could add the new one, and while testing, it would test from most-to-least recent.

I check around, and that would only be possible with some custom methods and/or hacks

I have another suggestion though. Cypress recommends to use data selectors. We could enforce this. That way if you change something in your app, just make sure to keep using the same data selector and you'll be fine across versions! :rocket:

e.g.

// ----- Imported from @nextcloud/cypress
// return main file list
const fileList = () => cy.get('[data-filelist]')
// return file row for ID within main file list
const fileSelector = ({ id }) => cy.getNcElement(fileList).find(`[data-file-id=${id}]`)

// -----  Your cypress test
cy.getNcElement(fileSelector, { id: fileInfo.id })
    .should('exist')
// or with the fileInfo straight away
cy.getNcElement(fileSelector, fileInfo)
    .should('exist')
skjnldsv commented 2 years ago

image Works nicely!

skjnldsv commented 1 year ago

Addressed, for followup discussion/questions/enhancements, see https://github.com/nextcloud/nextcloud-cypress