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

Toolchain for Test task #3064

Merged
merged 6 commits into from Jul 19, 2023
Merged

Toolchain for Test task #3064

merged 6 commits into from Jul 19, 2023

Conversation

TWiStErRob
Copy link
Contributor

@TWiStErRob TWiStErRob commented Jul 16, 2023

Re-raise of TWiStErRob#1 in mockito repo.

A step towards #2898, from here we could create multiple test tasks as a followup, and remove the gradle property and use the task name as dynamic matrix element instead.

Testing: see section in #3062

Additionally I added fail(System.getProperty("java.home")) in a test to see if the JDK was as expected.

I tried to minimize the changes, that's why I created a separate PR to make it possible to configure all projects at once.

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should mention Fixes #<issue number> if relevant

@TWiStErRob TWiStErRob changed the title Toolchains Toolchain for Test task Jul 16, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (1fc4451) 85.45% compared to head (25958c5) 85.45%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #3064   +/-   ##
=========================================
  Coverage     85.45%   85.45%           
  Complexity     2888     2888           
=========================================
  Files           329      329           
  Lines          8801     8801           
  Branches       1093     1093           
=========================================
  Hits           7521     7521           
  Misses          992      992           
  Partials        288      288           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

.github/workflows/ci.yml Show resolved Hide resolved
gradle.properties Show resolved Hide resolved
gradle/test-launcher.gradle Show resolved Hide resolved
@TimvdLippe
Copy link
Contributor

Do you know why the code coverage has increased with this PR? Looking at the changes, I don't see a particular reason why they would cover more. Or is this the result of running on Java 17 for Android? If so, are we also missing coverage now? I didn't spot anything, but let's double check.

Great change btw. We should definitely start using toolchains and now it is clear what we are using when.

@TWiStErRob
Copy link
Contributor Author

Do you know why the code coverage has increased with this PR? Looking at the changes, I don't see a particular reason why they would cover more.

@TimvdLippe the coverage changed because master android CI failed, so coverage from instrumentation tests was not included in baseline the PR is compared against. They passed on this PR, so they're covered, that's the increase. If you re-run this job on master so it passes, it should equalize. Please do.

@TimvdLippe
Copy link
Contributor

@TWiStErRob can you rebase on master? I merged some Dependabot PRs and unfortunately they now clash. That should also fix the coverage issue, as I reran that job

Using a toolchain installed via auto-provisioning, but having no toolchain repositories configured. This behavior is deprecated. Consider defining toolchain download repositories, otherwise the build might fail in clean environments; see https://docs.gradle.org/8.2/userguide/toolchains.html#sub:download_repositories
@TWiStErRob
Copy link
Contributor Author

Clean rebase, only the trivial conflict with enterprise bump.

@TWiStErRob
Copy link
Contributor Author

TWiStErRob commented Jul 18, 2023

@TimvdLippe Coverage diff looks good. Do you agree with changes listed here? The biggest one is producing Java 11 bytecode on Java 17 for production code.

@TimvdLippe TimvdLippe merged commit cf8b820 into mockito:main Jul 19, 2023
13 checks passed
@TWiStErRob TWiStErRob deleted the toolchains branch July 19, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants