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/flaky react protractor #11923

Merged
merged 20 commits into from
Jun 21, 2020

Conversation

sendilkumarn
Copy link
Member

@sendilkumarn sendilkumarn commented Jun 10, 2020

This is another attempt to fix the flaky react protractor tests. The protractor best practice is to make the tests independent of each other, but here our tests are depending on each other. (check out style guide here).

But the flakiness might continue / resurface in the future(or even in this PR). The correct and long time solution will be to move towards Cypress.

MathieuAA and others added 14 commits June 10, 2020 08:27
Noticed issue: beforeRecordsCount was a global variable used in different async
functions, so not atomic. The tests weren't actually independent because of that.
This is an attempt to fix it.
Didn't see the ComponentsPage being initialized in the test!
Why simplified: because the deletion is actually done in steps. First, the
user clicks the entry deletion button, then the modal appears which hides the
table, the user clicks the button and the request is sent to the backend.
After that, the table is displayed and the toaster can appear before the
table is updated.

In the test, we ignore this fact and wait for the modal to disappear but
not for the table to be updated.
The test basically now says: okay we click the button, wait for the modal to
go away and then we wait for the table to update because there won't be any
entry after that.
If there's a backend issue, the wait will be over and the test will fail
because the last entry won't be deleted.
@DanielFran DanielFran closed this Jun 10, 2020
@DanielFran DanielFran reopened this Jun 10, 2020
@DanielFran
Copy link
Member

@sendilkumarn closed/reopened to validate if it was releated to jhispter bom commit

@sendilkumarn
Copy link
Member Author

@DanielFran it looks like it is working :)

@DanielFran
Copy link
Member

@sendilkumarn Yes. Need to check which dependency was the culpit...

@sendilkumarn sendilkumarn marked this pull request as ready for review June 10, 2020 22:07
@MathieuAA
Copy link
Member

@sendilkumarn it's gonna be a very busy day for me, but I'll try to review it ASAP

@MathieuAA
Copy link
Member

LGTM @sendilkumarn, I'm testing this against my weird fork

@sendilkumarn
Copy link
Member Author

yeah, I changed this on top of your draft PR 👍

@MathieuAA
Copy link
Member

Except for the usual memcached thing, everything seems to be okay @sendilkumarn. It should be good to go. Only 20 builds remaining

@MathieuAA
Copy link
Member

Okay, the memcached builds fail for the now known reason, there's also a backend test that fails because of a dependency issue. https://github.com/MathieuAA/private-generator-jhipster/actions

@MathieuAA
Copy link
Member

@sendilkumarn should your PR fail for the memcached builds? From what I see I'm not so sure

@sendilkumarn
Copy link
Member Author

It should not, can you link the failed builds?

@MathieuAA
Copy link
Member

@MathieuAA
Copy link
Member

Wait, why does it not fail here? perhaps there's something wrong with my repo

@sendilkumarn
Copy link
Member Author

It looks like there is a fake data, but this is interesting.

ignore checking any displayed multiple times
@pascalgrimaud
Copy link
Member

@sendilkumarn @MathieuAA : do you think we can merge this?my opinion is : it can be only similar or better than what we currently have

Don't hesitate to merge it !

@sendilkumarn sendilkumarn merged commit e77ebe8 into jhipster:master Jun 21, 2020
@sendilkumarn sendilkumarn deleted the fix/flaky-react-protractor branch June 21, 2020 00:44
@pascalgrimaud pascalgrimaud added this to the 6.10.0 milestone Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants