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

Remove ConsoleLauncher configuration in Ant starter (#98) #114

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

seachicken
Copy link

@seachicken seachicken commented Oct 2, 2019

Overview

Fixes #98
Fixes #115


I hereby agree to the terms of the JUnit Contributor License Agreement.

@sbrannen sbrannen changed the title Remove ConsoleLauncher configuration (#98) Remove ConsoleLauncher configuration in Ant starter (#98) Oct 2, 2019
Comment on lines 21 to 28
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/platform/junit-platform-commons/${junit_platform_version}/junit-platform-commons-${junit_platform_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/platform/junit-platform-engine/${junit_platform_version}/junit-platform-engine-${junit_platform_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/platform/junit-platform-launcher/${junit_platform_version}/junit-platform-launcher-${junit_platform_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/jupiter/junit-jupiter-api/${junit_jupiter_version}/junit-jupiter-api-${junit_jupiter_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/jupiter/junit-jupiter-engine/${junit_jupiter_version}/junit-jupiter-engine-${junit_jupiter_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/junit/jupiter/junit-jupiter-params/${junit_jupiter_version}/junit-jupiter-params-${junit_jupiter_version}.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/opentest4j/opentest4j/1.2.0/opentest4j-1.2.0.jar")
(cd "${ant_folder}/lib" && curl --remote-name "http://central.maven.org/maven2/org/apiguardian/apiguardian-api/1.1.0/apiguardian-api-1.1.0.jar")
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be easier to use Apache Ivy to retrieve the necessary dependencies instead of handling that manually with curl?

Copy link
Author

Choose a reason for hiding this comment

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

How about supporting Ivy when supporting Ant-wrapper at #66 ?

Copy link
Member

Choose a reason for hiding this comment

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

Why not keeping junit-platform-console-standalone JAR here? Does it no longer work with the <junitlauncher> task?

Copy link
Member

Choose a reason for hiding this comment

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

How about supporting Ivy when supporting Ant-wrapper at #66 ?

Does the Ant-wrapper make it somehow easier to use Ivy?

Copy link
Author

Choose a reason for hiding this comment

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

Why not keeping junit-platform-console-standalone JAR here? Does it no longer work with the <junitlauncher> task?

Ant allows testing to be launched, so junit-platform-console-standalone is not required.
https://ant.apache.org/manual/Tasks/junitlauncher.html

Copy link
Author

Choose a reason for hiding this comment

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

How about supporting Ivy when supporting Ant-wrapper at #66 ?

Does the Ant-wrapper make it somehow easier to use Ivy?

I don't know much the Ant-wrapper. Is it better to use ivy directly?

Copy link
Member

@sbrannen sbrannen Oct 2, 2019

Choose a reason for hiding this comment

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

In my opinion, it's always better to use Ant with Ivy, since Ivy provides dependency management similar to that provided by Maven or Gradle.

From the documentation for the Ant-wrapper, it appears that that's easier to set up if you're already using Ivy.

So maybe we should actually take the following steps.

  1. Use Ivy in the Ant examples.
  2. Use the Ant-wrapper in the Ant examples.
  3. Use the junitlauncher task in the Ant examples.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

I see, sounds good! but what does mean No.3? I already use this one fb685f4#diff-72ebd09ba035959b381f450ac03a68efR37

Copy link
Member

Choose a reason for hiding this comment

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

I meant that we should ideally follow those steps.

But... if we mix up the order a bit and still arrive at the same outcome, that works, too. 😉

@sbrannen sbrannen dismissed their stale review October 2, 2019 16:10

Let's push the use of Ivy to a separate issue

junit5-jupiter-starter-ant/build.sh Outdated Show resolved Hide resolved
Copy link
Member

@sormuras sormuras left a comment

Choose a reason for hiding this comment

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

@sbrannen May I consider your requested changes as resolved and merge this PR?

@marcphilipp
Copy link
Member

With two approvals and since I think his requested changes have been resolved, I think we can overrule @sbrannen this once.

@marcphilipp marcphilipp changed the base branch from master to main June 19, 2020 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Apache Ivy in Ant examples Remove ConsoleLauncher configuration in Ant starter
4 participants