Add a Cypress test for displaying edited events #9886
Conversation
MSC3925 is changing this, so let's make sure it keeps working.
richvdh
added
the
T-Task
|
The typescript check is mystifying me: it works locally, and |
cypress/e2e/editing/editing.spec.ts
Outdated
| cy.get(`[data-event-id="${originalEventId}"]`).then((messageTile) => { | ||
| // at this point, the edit event should still be unknown | ||
| cy.getClient().then((cli) => { | ||
| expect(cli.getRoom(testRoomId).getTimelineForEvent(editEventId)).to.be.null; |
There was a problem hiding this comment.
question for the reviewer: I'm not very happy with all this indentation; it feels like a return to the bad old days of javascript callback hell. Generally I feel like I only half-grok how Cypress is intended to be used :(. Suggestions as to how to make this better would be welcome.
There was a problem hiding this comment.
Instead of using then to handle the promise, can't we use here async/await to handle the promise flow ?
There was a problem hiding this comment.
const win = await cy.window({ log: false })
const bob = await cy.getBot(synapse, { displayName: "Bob", userIdPrefix: "bob_" })
...
cy.joinRoom(testRoomId)
...
const messageTile = await cy.get(`[data-event-id="${originalEventId}"]`)
const cli = await cy.getClient()
...What do you think about it ?
There was a problem hiding this comment.
I think it sounds good but doesn't work. The things returned by cy.* aren't real promises, so can't be awaited. cypress-io/cypress#1417 has a whole bunch to say about this.
There was a problem hiding this comment.
You might prefer to use an alias and then the cy.get("@aliasname").then(... to reduce nesting.
There was a problem hiding this comment.
I've managed to clean this up a fair bit, partly with the use of cy.all.
The previous implementation was indecipherable, and didn't actually work.
Restructure the test a bit to reduce the indentation.
because (a) that's cleaner (b) it might fix the rando typescript errors
* Hoist the getClient to the top level, to avoid the random `.wrap(null)` * Use .should rather than .then, because that's the cypress way
For the record, the problem was that my branch was out of date with respect to develop (specifically #9882, which removes the import of |
MSC3925 is changing this, so let's make sure it keeps working.
MSC3925 is changing this, so let's make sure it keeps working.
This change is marked as an internal change (Task), so will not be included in the changelog.