Skip to content

Execute only FIPS Unit tests, not UT of dependent modules - #22038

Merged
ahus1 merged 1 commit into
keycloak:mainfrom
mabartos:fips_ut
Aug 2, 2023
Merged

Execute only FIPS Unit tests, not UT of dependent modules#22038
ahus1 merged 1 commit into
keycloak:mainfrom
mabartos:fips_ut

Conversation

@mabartos

@mabartos mabartos commented Jul 27, 2023

Copy link
Copy Markdown
Member

Just spotted this issue.

As the -am is specified, all dependent modules are also built, which provoke also the execution of unit tests for dependent modules. The execution of other UTs is made in Base UT and doesn't need to be executed twice as there's no difference to execute it with various types of FIPS configs.

@rmartinc @mposolda Could you please check it? Thanks

@mabartos
mabartos requested a review from a team as a code owner July 27, 2023 15:20
@mabartos
mabartos requested a review from a team July 27, 2023 15:20
@ghost ghost added the team/core label Jul 27, 2023
@mposolda mposolda self-assigned this Jul 28, 2023
mposolda
mposolda approved these changes Jul 28, 2023
View reviewed changes

@mposolda mposolda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mabartos LGTM, Thanks for this improvement!
@rmartinc Does this look ok to you?

@mposolda
mposolda requested a review from rmartinc July 28, 2023 06:38
rmartinc
rmartinc approved these changes Jul 28, 2023
View reviewed changes

@rmartinc rmartinc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thanks @mabartos !

@mabartos

mabartos commented Jul 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

The Keycloak CI wasn't executed as it seems the conditional stuff doesn't catch changes in the scripts folder?

@mposolda Is it possible to execute it? Thanks

EDIT: Related to: #22064

@mposolda

mposolda commented Jul 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@mabartos It seems I cannot manually trigger it. The option might be one of:

  • Do some "dummy change" in this PR in some file under crypto/* to ensure the CI is triggered.
  • Do some "dummy change" just in some your branch (you can add dummy-change in additional commit) and push to your fork and then send the link here where it can be seen that FIPS tests are successfully executed
  • Wait until Execute CI when some changes were made in scripts dir #22064

@mabartos

mabartos commented Jul 28, 2023

Copy link
Copy Markdown
Member Author

@mposolda Ok, thanks. We can wait for the #22064

@mabartos

mabartos commented Jul 31, 2023

Copy link
Copy Markdown
Member Author

@mposolda Rebased. FIPS UT passed; everything works as expected.

@ahus1
ahus1 merged commit ee6a29f into keycloak:main Aug 2, 2023
stianst pushed a commit to stianst/keycloak that referenced this pull request Sep 4, 2023
@mabartos @stianst
Execute only FIPS Unit tests, not UT of dependent modules (keycloak#2…
a27aaa3 
…2038)
stianst pushed a commit that referenced this pull request Sep 4, 2023
@mabartos @stianst
Execute only FIPS Unit tests, not UT of dependent modules (#22038)
e6b8ae7
This was referenced Sep 6, 2023
Closed
Closed
@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

@mposolda mposolda mposolda approved these changes
@rmartinc rmartinc rmartinc approved these changes

Assignees

@mposolda mposolda

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mabartos @mposolda @rmartinc @ahus1