Skip to content

Attach js-adapter to maven uploads and rewrite version to semver compatible if needed#23512

Merged
stianst merged 1 commit intokeycloak:mainfrom
stianst:js-adapter-dist
Sep 27, 2023
Merged

Attach js-adapter to maven uploads and rewrite version to semver compatible if needed#23512
stianst merged 1 commit intokeycloak:mainfrom
stianst:js-adapter-dist

Conversation

@stianst
Copy link
Contributor

@stianst stianst commented Sep 25, 2023 •
edited by jonkoops
Loading

Updates set-version.sh to check if version is not semver compliant, and rewrites for NPM packages if needed.

For example 22.0.3.redhat-00001 will be rewritten to 22.0.3+redhat-00001.

Also attaches the JS adapter tarball to Maven, which makes it available in local repository with mvn install and also includes it in artifacts uploaded to Maven repository for a release.

Closes #23312

@stianst stianst requested review from a team as code owners September 25, 2023 12:04
@ghost ghost added team/ui labels Sep 25, 2023
@stianst stianst mentioned this pull request Sep 25, 2023
Closed
@cypress
Copy link

cypress bot commented Sep 25, 2023 •
edited
Loading

1 flaky test on run #9100 ↗︎

0 527 48 0 Flakiness 1

Details:

Merge a47073a into d90640b...
Project: Keycloak Admin UI Commit: ae753f7f8f ℹ️
Status: Passed Duration: 12:07 💡
Started: Sep 26, 2023 11:40 AM Ended: Sep 26, 2023 11:52 AM
Flakiness  cypress/e2e/realm_settings_events_test.spec.ts • 1 flaky test • chrome

View Output Video

Test Artifacts
Realm settings events tab tests > Should remove event from event listener Test Replay Output Screenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@stianst stianst mentioned this pull request Sep 25, 2023
Closed
jonkoops
jonkoops previously approved these changes Sep 25, 2023
View reviewed changes
pskopek
pskopek requested changes Sep 26, 2023
View reviewed changes
set-version.sh Outdated
Copy link
Contributor

@pskopek pskopek Sep 26, 2023

Choose a reason for hiding this comment

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

This doesn't produce valid semantic version.
E.g. 22.0.3.redhat-00001 -> 22.0.3.redhat+00001
Which is not valid semantic version. Valid would look like 22.0.3+redhat-00001
(check here: https://regex101.com/r/ETSiwK/1)

I suggest to use: NEW_NPM_VERSION=$(echo $NEW_VERSION | awk -F '.' '{ print $1"."$2"."$3"+"$4 }')

Copy link
Contributor Author

@stianst stianst Sep 26, 2023

Choose a reason for hiding this comment

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

Tried to avoid that since it would convert 22.0.0-alpha to 22.0.0+alpha in case we ever wanted to do a pre-release, but we can fix that if we ever do it (which is unlikely tbh).

Copy link
Contributor

@jonkoops jonkoops Sep 27, 2023

Choose a reason for hiding this comment

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

Ahh yes, I overlooked this. Looks like this has now been changed to 22.0.3+redhat-00001 so that should be valid.

pskopek
pskopek approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@pskopek pskopek left a comment

Choose a reason for hiding this comment

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

LGTM!

@ghost ghost added the flaky-test label Sep 26, 2023
@ghost ghost mentioned this pull request Sep 26, 2023
Closed
@ghost
Copy link

ghost commented Sep 26, 2023

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.ui.account2.WelcomeScreenTest#applicationsTest

Keycloak CI - Account Console IT (firefox)

org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a org.keycloak.testsuite.util.URLAssert URL expected to begin with: https://localhost:8543/auth/realms/test/protocol/openid-connect/auth ; actual URL: https://localhost:8543/auth/realms/test/account/#/applications within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:985)
...

Report flaky test

ghost
ghost reviewed Sep 26, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@edewit
Copy link
Contributor

edewit commented Sep 26, 2023

@stianst isn't it an option to use something like mvnpm.org to get this?

@stianst
Copy link
Contributor Author

stianst commented Sep 26, 2023

@stianst isn't it an option to use something like mvnpm.org to get this?

Nah, we actually need it the other way around.

@stianst stianst merged commit 1c28c6c into keycloak:main Sep 27, 2023
stianst added a commit to stianst/keycloak that referenced this pull request Sep 27, 2023
@stianst
Attach js-adapter to maven uploads and rewrite version to semver comp…
fa0d4b0 
…atible if needed (keycloak#23512)

Closes keycloak#23312
@stianst stianst deleted the js-adapter-dist branch September 27, 2023 09:27
stianst added a commit that referenced this pull request Sep 27, 2023
@stianst
Attach js-adapter to maven uploads and rewrite version to semver comp…
2f67aa1 
…atible if needed (#23512) (#23555)

Closes #23312
@ASzc ASzc mentioned this pull request Oct 2, 2023
Closed
2 tasks
@stianst stianst mentioned this pull request Nov 14, 2023
Closed
@stianst stianst mentioned this pull request May 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pskopek pskopek pskopek approved these changes

@ASzc ASzc Awaiting requested review from ASzc

+2 more reviewers

@jonkoops jonkoops jonkoops approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deploy JavaScript adapter to Maven repository

4 participants

@stianst @edewit @pskopek @jonkoops