-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
Run cypress test at workflow. #225
Conversation
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@mshima Is this still needed? |
Reopened to create a new merge point. |
3 of 7 tests are passing if the account register is disabled.
Looks like its using hibernate?? |
Any specific reason for that?
That's just Validator. Actual cause could be mismatch in impl vs expectation. Another point I remember about OIDC flow is that it requires a minor override in cypress login command to redirect post authentication ( at least for Keycloak). There are other open issues like missing authorities endpoint, missing roles etc that require closure before achieving a green state. |
Disable account register? |
For oauth2 we could try to migrate to ui login. |
Did you try with latest code? |
I haven't observed much issues with login post initial implementation. We can try login with UI (with all providers), however that should be done on JHipster side and not over here. For current issue, the change is at a single place to redirect (current conf is to not do that) |
Sure https://github.com/jhipster/generator-jhipster-quarkus/actions/runs/3299590773 (last but one commit) With account registration disabled https://github.com/jhipster/generator-jhipster-quarkus/actions/runs/3299677293 (last commit) |
strategy: | ||
fail-fast: false | ||
matrix: | ||
node_version: [14.16.0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should update to align with supported node version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, at jhipster we read from constants now.
- app: imperative-sql-jwt-maven-no-db | ||
- app: imperative-ngx-oauth2-maven-mysql-caffeine | ||
exclude: | ||
- app: imperative-ngx-jwt-gradle-mongodb-redis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the exclusion is due to some failures, then, please log an issue so that it can be looked into and fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's excluded from main workflow too. That's the reason.
- name: 'SETUP: Checkout generator-jhipster' | ||
uses: actions/checkout@v2 | ||
with: | ||
repository: 'jhipster/generator-jhipster' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we are relying on the main branch of JHipster
. I suggest to rely on the current supported JHipster version i.e. v7.9.3
to avoid any surprises.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It should be version specific.
generators/cypress/index.js
Outdated
get postWriting() { | ||
return { | ||
customize() { | ||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove commented code block
generators/cypress/index.js
Outdated
"it.skip('should be able to init reset password'" | ||
); | ||
*/ | ||
this.replaceContent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take a look into registration test failure. Can you remove this hack?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, fell free to change the PR or create a branch in the this repository as you wish.
with: | ||
path: generator-jhipster-quarkus | ||
fetch-depth: 5 | ||
- name: git history |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Displaying git history in CI seems just clutter and not much useful. Can we remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can be dropped.
id: compare | ||
if: steps.base-app.outcome == 'success' | ||
uses: ./generator-jhipster/.github/actions/compare | ||
- name: 'Backend diff with updated prettier' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v6 -> v7 was complicated and all files changed due to prettier changes.
It was temporary added for this purpose. Can be dropped.
@@ -0,0 +1,124 @@ | |||
name: Imperative Smoke Tests Compare |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having two CI workflows executing similar tests, can we enrich existing CI tests to add compare functionality and to run the e2e tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t recall exactly why, but they were temporary created for different purposes for the v6 -> v7 migration.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Draft a custom test with cypress.
Disabled 2 tests to succeed for 1 test to succeed.
Broken: