[GitHub] [ant-ivy] jaikiran commented on pull request #96: IVY-1632: Use valid value for HTTP header "Accept".

[GitBox]

jaikiran commented on pull request #96:
URL: https://github.com/apache/ant-ivy/pull/96#issuecomment-130597


   This looks OK to me. However, it looks like this PR didn't trigger a CI run, 
so I'll manually run our tests to make sure this doesn't break anything, before 
merging this.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] [ant] jaikiran commented on pull request #173: Allow ant:get task to disable authentication on redirect.

[GitBox]

jaikiran commented on pull request #173:
URL: https://github.com/apache/ant/pull/173#issuecomment-129159


   >Therefore I didn't change the default behavior to avoid breaking existing 
Ant scripts. This means, "authenticateOnRedirect" defaults to "true". But maybe 
it would be better to change this.
   
   I was leaning towards making this new `authenticateOnRedirect` to default to 
`false` to be more secure (i.e. don't set Authorization header to redirected 
URL unless explicitly asked to). That might break scripts but I think that's 
probably a good thing since it would force users to review their target URLs 
and decide if they really want to send the auth header on redirect for that 
specific URL.
   
   The only place where this would probably be a nuisance is if the redirect is 
happening just for the scheme. What I mean is if the original URL 
`http://example.com/foo` was redirecting to `https://example.com/foo`. Or even 
in some cases where servers redirect a URL of the form `http://example.com/foo` 
to `http://example.com/foo/` (slash at the end). So yes, I guess leaving the 
current backward compatible behaviour is OK.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Re: [DISCUSS] JUnit 5 Customization

[Jaikiran Pai]

Hello Aleksei,

On 22/12/21 7:59 pm, Aleksei Zotov wrote:

Hi Ant Developers,

I'm working on the migration of Apache Cassandra project from JUnit 4 to
JUnit 5 (CASSANDRA-16630
) which turned out
to be larger than I originally expected. At the moment, three PRs got
merged:

- https://github.com/apache/ant/pull/147
- https://github.com/apache/ant/pull/168
- https://github.com/apache/ant/pull/172

And there are a few more I'd like to discuss with you.


Sorry, I remember seeing a PR from you where you asked for some inputs 
in this area, but I haven't been able to get to it due to some other 
priorities.




Formatters Extensibility
Apache Cassandra extended the existing JUnit 4 formatters in order to
integrate them with a custom logic required for a better CI process. It was
easily achievable for JUnit 4 since the formatters are marked public. On
the contrary, for JUnit 5 all formatters are package private. Even
*AbstractJUnitResultFormatter* is package private. Of course, we can
copy-paste all these classes to our project and customize them based on our
needs, but that seems to be a bit of an overhead.

Currently, we'd like to make *AbstractJUnitResultFormatter *extensible.
Here is the corresponding PR: https://github.com/apache/ant/pull/169/


The reason why these formatters are package private and not extensible 
is intentional. When I added these formatters, one of the goals of these 
formatters was to only make them generate output to an extent that it 
matches the JUnit4 style formatters of the "junit" task (hence the name 
"legacy-" to those formatters). All these pre-shipped "legacy-" 
listeners that you see in Ant are only there for minimal support and 
their implementation is considered to be internal to the Ant distribution.


The new JUnit5 framework itself is extensible and also comes with some 
pre-shipped "listeners" (implementations of TestExecutionListener). 
That's one of the reasons why the junitlauncher task's "listener" 
element provides a direct way to use any implementation of the JUnit5 
framework's "org.junit.platform.launcher.TestExecutionListener". i.e. 
for any code that seeks extensibility, the 
org.junit.platform.launcher.TestExecutionListener is expected to be the 
extension point/interface and not Ant's internal 
AbstractJUnitResultFormatter. This allows the junitlauncher task to be 
minimal and allows it to achieve its goal of just being something that 
will launch the JUnit5 framework.


I can understand that some of the code in the 
AbstractJUnitResultFormatter might appear reusable and would be tempting 
to reuse/extend, but I wouldn't want anything outside of Ant to start 
using these classes and instead just code against the JUnit5 
classes/interfaces.


Having said all this, if any of other maintainers of the Ant project 
feel that we should allow these internal classes to be extensible, I 
won't mind having this work reviewed and merged.



Fork Mode Support
Apache Cassandra is a large and complex product and in order to guarantee
its quality we run many tests independently. It lets us ensure different
test suites do not affect each other. For isolated testing we spin up a new
JVM per tests suite via *forkMode* property. Unfortunately,
*junitlauncher* task
does not provide such a functionality.

Currently, we'd like *mode* attribute to *fork* element. Here is the
corresponding PR: https://github.com/apache/ant/pull/174


I remember there was some bugzilla discussion of a similar question (or 
maybe on some other forum). I'll take a look at this PR soon.



I'd be glad if you could provide some feedback on that. Also I need some
guidance here - I have a suspicion that *JUnitLauncherTaskTest* is not
being run during "./build clean test" (I cannot grep it in logs). Could you
please help me to run this particular test from CLI.


Have you fetched the optional JUnit5 libraries before running these 
tests? A lot of Ant's tasks are optional and their tests are only run if 
the corresponding libraries are available in the classpath. To fetch 
these dependencies you can do:


ant -f fetch.xml -Ddest=optional

and then run the tests using the command you currently use.


Use File Support
Apache Cassandra does not always need to write testing output to a file.
Even though it is possible to achieve writing into standard output (by
overriding *TestResultFormatter.setDestination *method) instead of a file,
it is impossible to prevent empty files from being created on the file
system. Of course, we can delete these files, however, it would be nice to
have the functionality similar to *junit* task.

I've already started a related discussion on the PR (please, chime in):
https://github.com/apache/ant/pull/171


I'll take a look at this one.

-Jaikiran


-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: 

[GitHub] [ant] bodewig commented on pull request #173: Allow ant:get task to disable authentication on redirect.

[GitBox]

bodewig commented on pull request #173:
URL: https://github.com/apache/ant/pull/173#issuecomment-999741812


   many thanks @bernolanger
   
   we'd like to credit you in CONTRIBUTORS and contributors.xml. What is the 
name you'd want us to use?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] [ant] bodewig merged pull request #173: Allow ant:get task to disable authentication on redirect.

[GitBox]

bodewig merged pull request #173:
URL: https://github.com/apache/ant/pull/173


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[DISCUSS] JUnit 5 Customization

[Aleksei Zotov]

Hi Ant Developers,

I'm working on the migration of Apache Cassandra project from JUnit 4 to
JUnit 5 (CASSANDRA-16630
) which turned out
to be larger than I originally expected. At the moment, three PRs got
merged:

   - https://github.com/apache/ant/pull/147
   - https://github.com/apache/ant/pull/168
   - https://github.com/apache/ant/pull/172

And there are a few more I'd like to discuss with you.

Formatters Extensibility
Apache Cassandra extended the existing JUnit 4 formatters in order to
integrate them with a custom logic required for a better CI process. It was
easily achievable for JUnit 4 since the formatters are marked public. On
the contrary, for JUnit 5 all formatters are package private. Even
*AbstractJUnitResultFormatter* is package private. Of course, we can
copy-paste all these classes to our project and customize them based on our
needs, but that seems to be a bit of an overhead.

Currently, we'd like to make *AbstractJUnitResultFormatter *extensible.
Here is the corresponding PR: https://github.com/apache/ant/pull/169/

I'd be happy to make other existing formatters extensible as well. Please,
let me know if that sounds good to you and I'll add the changes to the
existing PR or raise another one.

Fork Mode Support
Apache Cassandra is a large and complex product and in order to guarantee
its quality we run many tests independently. It lets us ensure different
test suites do not affect each other. For isolated testing we spin up a new
JVM per tests suite via *forkMode* property. Unfortunately,
*junitlauncher* task
does not provide such a functionality.

Currently, we'd like *mode* attribute to *fork* element. Here is the
corresponding PR: https://github.com/apache/ant/pull/174

I'd be glad if you could provide some feedback on that. Also I need some
guidance here - I have a suspicion that *JUnitLauncherTaskTest* is not
being run during "./build clean test" (I cannot grep it in logs). Could you
please help me to run this particular test from CLI.

Use File Support
Apache Cassandra does not always need to write testing output to a file.
Even though it is possible to achieve writing into standard output (by
overriding *TestResultFormatter.setDestination *method) instead of a file,
it is impossible to prevent empty files from being created on the file
system. Of course, we can delete these files, however, it would be nice to
have the functionality similar to *junit* task.

I've already started a related discussion on the PR (please, chime in):
https://github.com/apache/ant/pull/171


I'd be very grateful to get code review comments on the PRs and your
feedback on the above points.


Best Regards,

Aleksei Zotov.

[GitHub] [ant] azotcsit commented on a change in pull request #169: junitlauncher - Improved extensibility capabilities for formatters

[GitBox]

azotcsit commented on a change in pull request #169:
URL: https://github.com/apache/ant/pull/169#discussion_r773913282



##
File path: 
src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/AbstractJUnitResultFormatter.java
##
@@ -43,7 +43,7 @@
 /**
  * Contains some common behaviour that's used by our internal {@link 
TestResultFormatter}s
  */
-abstract class AbstractJUnitResultFormatter implements TestResultFormatter {
+public abstract class AbstractJUnitResultFormatter implements 
TestResultFormatter {

Review comment:
   It is impossible to have abstract class protected, so it is marked as 
public. The methods are protected, not public.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

[GitHub] [ant] azotcsit opened a new pull request #174: junitlauncher - Added fork mode support

[GitBox]

azotcsit opened a new pull request #174:
URL: https://github.com/apache/ant/pull/174


   ### Context
   I'm working on migration of Cassandra to JUnit5 
(https://issues.apache.org/jira/browse/CASSANDRA-16630). Our test running code 
is heavily customized and relies onto 
[forkMode](https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L185)
 property. Unfortunately, _junitlauncher_ task does not have any equivalent. 
The purpose of this change is to introduce a way to run tests independently (in 
separate JVMs).
   
   ### Behavior
   This PR does not change any existing behavior. By default, the tests will be 
run in the same forked JVM (aka mode="once").
   
   ### Summary of the changes
   1. Added `node` attribute to `fork` element
   2. Added logic to spin a JVM per test suite class into `JUnitLauncherTask`
   3. Updated tests and documentation
   
   **I'm not sure `JUnitLauncherTaskTest` is run during `./build clean test`, 
hence the test might be broken.**
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



-
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org