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.

Sign up for GitHub

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

Jump to bottom

Fix gradle - junit5 integration templates and instructions #4600

Merged
merged 4 commits into from
Oct 17, 2019
Merged

Fix gradle - junit5 integration templates and instructions #4600

merged 4 commits into from
Oct 17, 2019

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Oct 16, 2019

No description provided.

@geoand geoand mentioned this pull request Oct 16, 2019
Closed
@geoand geoand force-pushed the junit-platform-gradle branch from e41b501 to 918d3e7 Compare October 16, 2019 12:02
@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019

@gsmet This should be ready now.
@gastaldi Mind giving it a spin as well?

@geoand geoand added this to the 0.26.0 milestone Oct 16, 2019
@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019

The regular tests can be run using ./gradlew test while the native tests run using ./gradlew check (or ./gradle nativeTest)

@gastaldi
Copy link
Contributor

gastaldi commented Oct 16, 2019 •
edited
Loading

It would be nice if the CreateProject also generated a README.md file with instructions on how to build the project.

I'll create a separate issue for that

@gastaldi
Copy link
Contributor

There you go: #4608

@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019

Nice idea!

@gastaldi
Copy link
Contributor

@geoand it looks like the native image is always built when you gradle build the project, is that intended?

@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019 •
edited
Loading

@gastaldi good question... It's a by-product of having the check depend on nativeTest.

So the question is: Should we do what we do with Maven and hide the native stuff behind a flag (no profiles in Gradle) or have users use Gradle's -x buildNative nativeTest when they don't want the integration tests (I would assume this is more intuitive to gradle users).

@gastaldi
Copy link
Contributor

I think option 1 would be more like it, but I'd like to hear more from actual Gradle plugin users

@gastaldi
Copy link
Contributor

Not opposed to any of those options btw

@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019

@Sanne WDYT? Is the more Gradle friendly option to hide tasks behind flags or to leave it up to the users to execute -x .... for whatever they don't want?

@gastaldi
Copy link
Contributor

btw, nativeTest should be renamed to testNative to match buildNative :)

@geoand geoand force-pushed the junit-platform-gradle branch from 918d3e7 to fe32bd9 Compare October 16, 2019 14:25
@geoand
Copy link
Contributor Author

geoand commented Oct 16, 2019

@gastaldi good point, done.

@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2019

@Sanne @gastaldi @gsmet The PR has now been updated to generate a clean build.gradle file.
The configuration logic that was needed to set things up has now been moved into the Gradle plugin itself.

@geoand geoand force-pushed the junit-platform-gradle branch 2 times, most recently from 337f97c to 9622c7e Compare October 17, 2019 08:17
@geoand geoand force-pushed the junit-platform-gradle branch from 9622c7e to a70bab1 Compare October 17, 2019 11:03
@geoand geoand force-pushed the junit-platform-gradle branch 2 times, most recently from 1c6a293 to efbe420 Compare October 17, 2019 11:16
@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2019

Arg.... Gradle has more surprises in store... Something is not working after the rename 🤦‍♂️

@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2019 •
edited
Loading

@gastaldi actually using testNative... for the source set is not working. It's probably some obscure gradle issue. I propose we don't spend any more time on it and get this in as is since currently generated gradle projects can't run tests.

@geoand geoand force-pushed the junit-platform-gradle branch from efbe420 to e23ecea Compare October 17, 2019 12:00
@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2019

OK, the final version has been pushed

@geoand
Copy link
Contributor Author

geoand commented Oct 17, 2019

The CI failure is known. Can I have a review of this?

gsmet
gsmet requested changes Oct 17, 2019
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Can you take care of my comment?

@geoand geoand force-pushed the junit-platform-gradle branch from e23ecea to 44ba936 Compare October 17, 2019 13:49
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 17, 2019
@geoand geoand merged commit 1228b4b into quarkusio:master Oct 17, 2019
@geoand geoand deleted the junit-platform-gradle branch October 17, 2019 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sanne Sanne Sanne left review comments

@gastaldi gastaldi gastaldi left review comments

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Milestone
0.26.0
Development

Successfully merging this pull request may close these issues.
4 participants
@geoand @gastaldi @Sanne @gsmet