Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

Update Gradle Examples #220

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

Update Gradle Examples #220

wants to merge 3 commits into from

Conversation

roguexz
Copy link
Contributor

@roguexz roguexz commented Dec 9, 2019

  • Update the build scripts to take advantage of the newer features in the plugin
  • Enable the examples project as part of the root project
  • Upgrade the Gradle wrapper to 6.0.1

* Update the build scripts to take advantage of the newer features in the plugin
* Enable the examples project as part of the root project
* Upgrade the Gradle wrapper to 6.0.1
@roguexz
Copy link
Contributor Author

roguexz commented Dec 9, 2019

Hopefully, we should get past the previous issue that surfaced. I still need to work on improving the ArtifactResolvingHelper when working with Gradle based projects.

@roguexz
Copy link
Contributor Author

roguexz commented Dec 9, 2019

Will look at the gradlew issue for Windows. Want to see if the linux build goes through or not.

@roguexz
Copy link
Contributor Author

roguexz commented Dec 9, 2019

@Ladicek A little help please. Looks like the Gradle repository is no longer available,

Could not find org.gradle:gradle-tooling-api:6.0.1.
  Searched in the following locations:
    - file:/root/.m2/repository/org/gradle/gradle-tooling-api/6.0.1/gradle-tooling-api-6.0.1.pom
    - https://repo.maven.apache.org/maven2/org/gradle/gradle-tooling-api/6.0.1/gradle-tooling-api-6.0.1.pom
    - https://oss.sonatype.org/content/repositories/snapshots/org/gradle/gradle-tooling-api/6.0.1/gradle-tooling-api-6.0.1.pom

@@ -11,7 +11,7 @@
<parent>
<groupId>io.thorntail.examples</groupId>
<artifactId>examples-parent</artifactId>
<version>2.5.1.Final-SNAPSHOT</version>
<version>2.6.1.Final-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, thanks :-) This didn't update because the module was commented out and I forgot about that.

@Ladicek
Copy link
Contributor

Ladicek commented Dec 10, 2019

I wonder why it's not looking into our Nexus...

@roguexz
Copy link
Contributor Author

roguexz commented Dec 10, 2019

Interesting, what would the URL for your internal Nexus repository be?

@Ladicek
Copy link
Contributor

Ladicek commented Dec 10, 2019

I added a commit to your PR that should fix the problem. At least it fixed the problem for me locally :-)

@roguexz
Copy link
Contributor Author

roguexz commented Dec 10, 2019

Interesting. Let me look at resolving the current problem. It seems related to the my next round of work, which is using an appropriate ArtifactResolvingHelper that searches for artifacts in the right location. I might be doing a crossover between ShrinkWrapARH & the CacheARH that is used by the runner.

@Ladicek
Copy link
Contributor

Ladicek commented Dec 10, 2019

I wonder why it's taking 30 minutes to run...

Also, and I should have probably mentioned it in the PR to thorntail/thorntail, I'm thinking if we perhaps aren't going a bit too far with the Gradle changes. For example, unconditionally adding Arquillian to all tests might not be the best thing to do -- sometimes, you don't want to use Arquillian.

@roguexz
Copy link
Contributor Author

roguexz commented Dec 10, 2019

I wonder why it's taking 30 minutes to run...

By switching the resolution handler, I should be able to fix this problem.

Also, and I should have probably mentioned it in the PR to thorntail/thorntail, I'm thinking if we perhaps aren't going a bit too far with the Gradle changes. For example, unconditionally adding Arquillian to all tests might not be the best thing to do -- sometimes, you don't want to use Arquillian.

I agree. I thought about it over many days and was debating both sides of the argument. I finally decided to use the following points to make a decision

  1. Chances are high that developers will try to mimic project structure and examples as published in this repository
  2. I believe all tests within Thorntail are defined to use Arquillian. This is another strong point influencing developers that Thorntail works really well with Arquillian.
  3. Even I learnt Arquillian because of the above two points.

I am open to both options. Before my previous PR, we were pulling in Arquillian related dependencies explicitly. I anticipated that some folks might have concerns around the recent change, which is why I made it a simple one-liner to revert :)

@roguexz
Copy link
Contributor Author

roguexz commented Dec 11, 2019

I think, I have found the problem. Will raise a PR against Thorntail once the local build goes through fine.

@roguexz
Copy link
Contributor Author

roguexz commented Dec 20, 2019

Also, and I should have probably mentioned it in the PR to thorntail/thorntail, I'm thinking if we perhaps aren't going a bit too far with the Gradle changes. For example, unconditionally adding Arquillian to all tests might not be the best thing to do -- sometimes, you don't want to use Arquillian.

I was thinking of addressing this comment by ensuring that there was a clear separate of concern and then realized that I have battled with this question sometime in the past as well :)

So here is a quick gist of what's happening,

  1. There are two Gradle plugins that we ship in that jar file
    • The package plugin which was the original plugin that takes care of invoking the build tool and bundling the final artifact.
    • The thorntail-arquillian plugin which is responsible for setting the environment when attempting to use Arquillian as your testing framework.
  2. When a developer applies the package plugin, it implicitly applies the thorntail arquillian plugin as well.

So, effectively speaking, the previous change isn't necessarily forcing too much of stuff on to a project. What we can do is stop applying the plugin implicitly. That would mean that all developers using the Thorntail Gradle integration (which I am sure we can quite literally count on our finger tips 😄) will need to update their project files to include the arquillian plugin explicitly.

Do let me know if you would prefer adopting that approach so that the docs and the examples can be updated.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 13, 2020

We can rerun the tests here when a new snapshot is deployed into our Nexus. That should be in a few hours. I'll trigger the tests later today, or tomorrow at worst.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 13, 2020

I think we can keep the thorntail-arquillian as is. You're right that the number of users isn't very high :-) Out of curiosity, is there a way to opt out of the Arquillian handling? As in, it can be on by default, but if there was a way to turn it off, that would be good (nice to have, not a must, I guess).

@Ladicek
Copy link
Contributor

Ladicek commented Jan 13, 2020

retest this please

@roguexz
Copy link
Contributor Author

roguexz commented Jan 13, 2020

At the moment, there is no option. But with my next change I plan on introducing better handling for Gradle tests so that they move through faster. If that model goes through fine, then we can split it up and inform the few developers who have been using Gradle about it.

The current inheritance model for ArtifactResolvingHelper on the Arquillian front is complicating things a little bit. I will look at the failures and try figuring out what is going on. I will post my findings after my analysis.

@Ladicek
Copy link
Contributor

Ladicek commented Jan 14, 2020

Funny that "Linux: Uberjar" passed :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants