Initial page object implementation

[asnaith]

Relates to #1079

What I changed

• Renamed the integration folder that the tests reside in to tests
• Created new page object classes, this will be the place where we declare selectors for a page's unique elements. It will help make tests more readable and easier to update element identifiers.
• Added more unique element identifiers
• Updated the settings tests to use page objects
• Added basic tests for main navigation

Notes

• Apologies for the "several things at once" PR. It just made sense for this initial PR to be done together
• Did not update file-management.ts yet, can be worked on in a separate PR

[render]

Your Render PR Server URL is https://files-ui-stage-pr-1080.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c2qm7hoirho589q1c5d0.

[asnaith]

It might make sense to append all of our test files with with "-test" or "-spec" too (spec is the common terminology in the cypress world). Could make it easier to distinguish the test files from others when we're searching in vscode etc. Sound good?

[Tbaut]

Renamed the integration folder that the tests reside in to tests

This seems to be a convention for Cypress, which I took the template from. I don't really mind tbh, "tests" is fine.

It might make sense to append all of our test files with with "-test"

Sure

I'll check real quick if we can use an object rather than classes, it'd be cleaner not to have to call the new Class() at the top of each test.

[Tbaut]

It's really cool thank you so much for that.
I left a couple comments, but other than that it looks great

[Tbaut]

I took the liberty to push a commit directly to your branch to show how we can achieve the same as the classes, but without bothering with all boilerplate and initialization.

Unless you think classes are much better for a reason that I missed, I think it'd be better to use objects like this.

[asnaith]

Hey Thibaut! Thanks, I agree that your approach here looks simpler and cleaner but doesn't this rule out the ability to use inheritance?

My idea behind the classes is mainly around re-usability and maintainability. At the moment the page classes only contain element declarations but they will eventually hold custom functions (helpers / assertions etc) that receive parameters from the test layer.

All the individual classes extend from BasePage. BasePage will eventually hold a collection of generic re-usable page agnostic helper functions that we can pass parameters to and call on any (subclassed) page.

When there's functionality on a specific page that differs from the generic behaviour we can override inherited functionality in the subclass. Similarly specific page classes will also contain non-generic functions that are exclusive to that specific page only.

Perhaps there's another option where we can keep things simple and reduce some of the boilerplate / initialization but also have the inheritance ability that I mentioned?

Really keen to hear all viewpoints and suggestions, I want to make sure we go with an approach that everyone is happy with.

[FSM1]

I think that the argument for classes makes a lot of sense here

[Tbaut]

Good points, I have some answers to some, but not all.

doesn't this rule out the ability to use inheritance?

We can still have inheritance with the spread operator, like

const someNewObject = {
  ...someBaseObjectInherited,
  newField: (param) => { usingThisParam(param) }
}

they will eventually hold custom functions (helpers / assertions etc) that receive parameters from the test layer

If I understand you correctly, this is solved by what I presented above, you can call from the test layer someNewObject.newField(someParam) and pass someParam from the test layer.

Similarly specific page classes will also contain non-generic functions that are exclusive to that specific page only.

This is where I start loosing you and where classes outperform objects. I think I understand what you mean, but I fear we'd be losing ourselves in complex structures. I def. do not have your experience and I don't know how complex things can get.
A rule of thumb I'd like to follow is that we want to have tests as easy to read as possible, but also easy to write. I think we should avoid complex construction as much as possible, hence trying to keep things simple.

Ultimately this is your call, I'm happy with either solution.

[asnaith]

(posting to close the loop for anyone reading this).

@Tbaut Now we have clarified we can still use inheritance I will proceed with your solution as we discussed on google hangouts :)

[asnaith]

It might make sense to append all of our test files with with "-test" or "-spec" too (spec is the common terminology in the cypress world). Could make it easier to distinguish the test files from others when we're searching in vscode etc. Sound good?

I've gone ahead and appended "spec" to the test file names.

[FSM1]

This looks really great.

[Tbaut]

Couple more comments, but mostly nits. Feel free to merge once you're happy with it and addressed (or not) my comments.

[Tbaut]

This before here is raising questions to me. If we put a .only to the second test it.only("can add/remove files and upload" below, then we'll be login-in twice am I right?

This before means the tests must run exactly in this order, which is not a good practice according to cypress.

I am not 100% sure why the before assertion was ever created by Cypress tbh, but having the before, and then seeing in the tests some other cy.web3Login({ clearCSFBucket: true }) make me think we shouldn't use it. Also I think it just makes things harder to read.

Let me know your thoughts.

[asnaith]

Removed :)

[Tbaut]

Do you know if the before is fired before the breforeEach? I see we're clicking the "hamburgerMenu" at the beginning of each test and wondering if we could have this in the beforeEach.

edit: yup before is fired first, so we may consider having the hamburger click in the beforeEach. cf cypress Hooks doc

[asnaith]

@Tbaut Good point. I'll update.

[Tbaut]

FYI, I just saw the test failing for "can rename a file with error handling".. it didn't fail locally and relaunching was fine, also a hint, for anyone following this conversation, to know where to find the screenshot when a test fails:

screenshot: