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(kotlin-dsl): unable to use events = setOf() in testLogging #28798

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nbrugger-tgm
Copy link

Context

It fixes a bug that prevents to use evennts = setOf() which would be the natural approach for Kotlin DSL. (since EnumSet.of() throws an exception on empty sets)

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes. (no user facing change)
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

Signed-off-by: Nils Brugger <nilsbrugger.nb.nb.nb@gmail.com>
@nbrugger-tgm nbrugger-tgm requested a review from a team as a code owner April 12, 2024 10:11
@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Apr 12, 2024
@tresat
Copy link
Member

tresat commented Apr 12, 2024

Thank you for your contribution!

Can you please add some tests to demonstrate and cover your changes? Is there any particular scenario you are looking to support?

@ov7a ov7a removed the to-triage label Apr 15, 2024
@nbrugger-tgm
Copy link
Author

nbrugger-tgm commented Apr 18, 2024

Added a unit test.
Its a bug fix so i do not know too well what "particular scenario" i should provide. Using an empty set on the setEvents method (default in kotlin dsl) crashes with a Java internal error from EnumSet.

The reason one wants to set an empty setEvents is when monitoring tests and so on is purely done via test reports (html/xml) and you don't want the "spam" of failed tests in the cli

PS: sorry for the delayed answer

Copy link

gitstream-cm bot commented Apr 18, 2024

Change Summary

This PR is 85.71% new code.
Platform Added Lines % of Total Line Changes Deleted Lines % of Total Line Changes Files Changed % of Total Files Changed
bt_ge_build_cache 0 0% 0 0% 0 0%
build_infrastructure 0 0% 0 0% 0 0%
core_configuration 0 0% 0 0% 0 0%
core_execution 0 0% 0 0% 0 0%
core_runtime 0 0% 0 0% 0 0%
documentation 0 0% 0 0% 0 0%
extensibility 0 0% 0 0% 0 0%
gradle_enterprise 0 0% 0 0% 0 0%
ide 0 0% 0 0% 0 0%
jvm 0 0% 0 0% 0 0%
kotlin_dsl 0 0% 0 0% 0 0%
release_coordination 0 0% 0 0% 0 0%
software 6 85.71% 1 14.29% 2 100%

Signed-off-by: Nils Brugger <nilsbrugger.nb.nb.nb@gmail.com>
@nbrugger-tgm
Copy link
Author

@tresat

@tresat tresat self-assigned this May 8, 2024
@tresat tresat added this to the 8.9 RC1 milestone May 8, 2024
@tresat tresat added the a:bug label May 8, 2024
@nbrugger-tgm nbrugger-tgm requested a review from a team as a code owner May 8, 2024 14:45
@tresat
Copy link
Member

tresat commented May 8, 2024

@bot-gradle test and merge

@bot-gradle bot-gradle added this pull request to the merge queue May 8, 2024
@bot-gradle
Copy link
Collaborator

The merge queue build has failed. Click here to see all failures.

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug from:contributor PR by an external contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants