Flaky Protractor tests with OAuth2

pascalgrimaud commented 3 years ago

Overview of the issue

Since this PR https://github.com/jhipster/generator-jhipster/pull/13530, the Protractor tests failed most of the time. See:

cc @mraible

protractor_monolith_oauth2 protractor_nodatabase_oauth2

Motivation for or Use Case

Reproduce the error

Related issues

Suggest a Fix

JHipster Version(s)

Current master

JHipster configuration

Entity configuration(s) `entityName.json` files generated in the `.jhipster` directory

Browsers and Operating System

[ ] Checking this box is mandatory (this is just to show you read everything)

mraible commented 3 years ago

I replaced some browser.sleep() calls with browser.wait(). Maybe that’s the problem?

https://github.com/jhipster/generator-jhipster/pull/13530/files#diff-373f2ed85b9eda1c4f074c4af4ef9ba41736e1fa41c2171e0187157a6634f4c8L90

On Jan 26, 2021, at 12:05 AM, Pascal Grimaud notifications@github.com wrote:

Overview of the issue

Since this PR #13530 https://github.com/jhipster/generator-jhipster/pull/13530, the Protractor tests failed most of the time. See:

https://github.com/hipster-labs/jhipster-daily-builds/actions?query=workflow%3A%22No+Database%22 https://github.com/hipster-labs/jhipster-daily-builds/actions?query=workflow%3A%22No+Database%22 https://github.com/hipster-labs/jhipster-daily-builds/actions?query=workflow%3A%22Monolith+OAuth2%22 https://github.com/hipster-labs/jhipster-daily-builds/actions?query=workflow%3A%22Monolith+OAuth2%22 cc @mraible https://github.com/mraible https://user-images.githubusercontent.com/9156882/105811397-e1120400-5fac-11eb-8ce5-efa53c970ed8.png https://user-images.githubusercontent.com/9156882/105811402-e2dbc780-5fac-11eb-93ad-d033f7e8393e.png Motivation for or Use Case

Reproduce the error

Related issues

Suggest a Fix

JHipster Version(s)

Current master

JHipster configuration

Entity configuration(s) entityName.json files generated in the .jhipster directory

Browsers and Operating System

Checking this box is mandatory (this is just to show you read everything) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/13723, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAELZAXMFUYY22L5KZJQCDS3ZSR3ANCNFSM4WS6WR4A.

pascalgrimaud commented 3 years ago

I don't know, I'm not a Protractor expert like you :) But we could give a try

pascalgrimaud commented 3 years ago

I'm reopening this, as it's still flaky nodatabase_flaky

mraible commented 3 years ago

I'll look into them now...

pascalgrimaud commented 3 years ago

I tried some change too, but didn't fix it yet.

My workflow:

create a local branch
commit some changes
push to my fork
then, launch No Database (or other build) at https://github.com/hipster-labs/jhipster-daily-builds/actions?query=workflow%3A%22No+Database%22
select my repo and select my branch : it's the most important part

mraible commented 3 years ago

I typically re-create the failing app locally, then run:

npm run ci:e2e:package
npm run ci:e2e:prepare
npm run ci:e2e:run

If it passes locally, it's a CI issue and we need to add pauses, wait fors, etc...

pascalgrimaud commented 3 years ago

Locally, it passes, I couldn't reproduce.

mraible commented 3 years ago

The first time I tried it, it didn't work.

[1]   2 passing (9s)
[1]   1 failing
[1]
[1]   1) Administration
[1]        "before all" hook: ret for "should load metrics":
[1]      TimeoutError: Wait timed out after 5003ms
[1]       at /Users/mraible/dev/react-nodatabase-oauth2-gradle/node_modules/selenium-webdriver/lib/promise.js:2201:17
[1]       at runMicrotasks (<anonymous>)
[1]       at processTicksAndRejections (internal/process/task_queues.js:93:5)

I tried again, and it did pass. Can we change the e2e script to try npm run ci:e2e:run multiple times?

I saw this recently with Protractor when running e2e tests on generated entities. It'd always pause on the edit screens. If I canceled and tried again, it'd pass on that edit screen, then pause/fail on the next one. If you canceled and restarted once for each generated entity, all tests eventually pass.

mraible commented 3 years ago

Are these running with JHI_E2E_HEADLESS=true? That might be an option. Or use another browser, like Firefox. I just tried and it works (from protractor.conf.js):

capabilities: {
  browserName: 'firefox',
},

NOTE: I did have to run npx webdriver-manager update after changing the browserName value.

mraible commented 3 years ago

Here's an attempted fix: https://github.com/jhipster/generator-jhipster/pull/13754.

pascalgrimaud commented 3 years ago

That's one of the reason we decided to use Cypress. Protractor is so random

Another solution would be to completely migrate to Cypress in Daily builds, and definitively abandon Protractor there

mraible commented 3 years ago

I'd be OK with that.

On Thu, Jan 28, 2021 at 16:46 Pascal Grimaud notifications@github.com wrote:

That's one of the reason we decided to use Cypress. Protractor is so random

Another solution would be to completely migrate to Cypress in Daily builds, and definitively abandon Protractor there

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/13723#issuecomment-769473181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAELZANWVCXINMGOO7DNALS4HZM3ANCNFSM4WS6WR4A .

mraible commented 3 years ago

If Cypress tests prove to be flaky, we can add another implementation in Playwright. 😜

pascalgrimaud commented 3 years ago

@mraible : for your information, I revert your PR here:

Then, I launch 5 NoDatabase builds consecutively. nodatabase_revert

So for me, there is something to do with your PR. If you're ok, we should revert it, and only add the username/password for Okta. The other codes should not changed.

mraible commented 3 years ago

I'm OK with a revert. None of these tests hit Okta and I tend to recommend Keycloak for CI anyway.

pascalgrimaud commented 3 years ago

Thanks, I'll take care of it tonight, after my day work

mraible commented 3 years ago

Woah! I didn't realize you want to revert all of my changes to support Okta. It works locally, most of the time (or at least, more than before). I'm OK with it for a release, but I'd love to collaborate with you on a blog post to explain the why. :)

mraible commented 3 years ago

BTW, I'm cool if your reason is "because Okta sucks!" We can do better. We should be better than Keycloak, IMHO.

pascalgrimaud commented 3 years ago

@mraible : sorry, I didn't explain well. Let me try again :-D

My branch with this commit https://github.com/pascalgrimaud/generator-jhipster/commit/0a5e996229a104baab01d37b7e4980bfe34100a8 is only here to confirm that the introduced code makes Protractor to become flaky -> this branch is only for test

I don't want to revert all the code. We should keep the username, password, and revert the other code (browser, element, etc), which seems not to be related directly to username/password

jhipster / generator-jhipster