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.

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] Remove failing tests #4473

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Conversation

jpelay
Copy link
Member

@jpelay jpelay commented Sep 12, 2023

In #4468 we merged a few cypress tests that had to do with the editor; they executed correctly in that PR, but after that they started failing consistently. So, in this PR I'm removing them from the test suite, leaving a comment stating that we need to come back to it, and also noting it in #4455.

Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

Yes, let's skip this for now, but I did place the issue on the meeting agenda, just so we don't forget!

@mergify
Copy link
Contributor

mergify bot commented Sep 13, 2023

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 138d7ce into hedyorg:main Sep 13, 2023
8 checks passed
@mergify
Copy link
Contributor

mergify bot commented Sep 13, 2023

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@jtwaleson
Copy link
Collaborator

@jpelay I was looking at the cause for the flakiness today. Two questions:

  • Maybe I'm missing something but if I look at the diff of this PR https://github.com/hedyorg/hedy/pull/4473/files there are no tests disabled, just a comment was added. Is this correct?
  • Were we ever able to reproduce the flakiness locally in cypress?

@Felienne
Copy link
Member

@jpelay I was looking at the cause for the flakiness today. Two questions:

Maybe you missed that is is a multiline comment, added around the test? That disables it. You can also see that after this PR the tests is no longer ran (see this page for info on how to run the tests: https://github.com/hedyorg/hedy/wiki/Hedy-Development-Process#local-installation) I hope that helps!

  • Were we ever able to reproduce the flakiness locally in cypress?

I have seen it fail locally too, and I think @jpelay has too, but I never dove in deeper.

@jpelay
Copy link
Member Author

jpelay commented Sep 17, 2023

Hi!

  • Were we ever able to reproduce the flakiness locally in cypress?

Yes, we were; the tests worked fine when I ran them in the Cypress GUI, but started failing when I used the command line instead. I fixed that locally by adding a hard coded wait after failing to find a different method to fix the problem in the amount of time that I decided to dedicate to this issue. Sadly, that wait failed also when we tried it on GitHub Actions.

However, locally, I managed to fix the tests without waits using the option --browser crhome that apparently is available on GitHub Actions, but I've yet to try that! What do you think about that option, would that be a good practice?

@jtwaleson
Copy link
Collaborator

Maybe you missed that is is a multiline comment, added around the test? That disables it. You can also see that after this PR the tests is no longer ran (see this page for info on how to run the tests: https://github.com/hedyorg/hedy/wiki/Hedy-Development-Process#local-installation) I hope that helps!

How silly of me, totally overlooked that! Sorry.

@Felienne
Copy link
Member

How silly of me, totally overlooked that! Sorry.

np, there are so many things to look at! Let us know if you need more help!

@jtwaleson
Copy link
Collaborator

Hi!

  • Were we ever able to reproduce the flakiness locally in cypress?

Yes, we were; the tests worked fine when I ran them in the Cypress GUI, but started failing when I used the command line instead. I fixed that locally by adding a hard coded wait after failing to find a different method to fix the problem in the amount of time that I decided to dedicate to this issue. Sadly, that wait failed also when we tried it on GitHub Actions.

However, locally, I managed to fix the tests without waits using the option --browser crhome that apparently is available on GitHub Actions, but I've yet to try that! What do you think about that option, would that be a good practice?

That helps, I'll dive into it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants