Re: [EXT] PR to move tests to src/test/groovy

["Milles, Eric (TR Technology) via dev" <de...@groovy.apache.org>]

Since this has moved forward, would you be able to apply the same to Groovy 4?  With this in place, moving commits (with tests) to older release branches requires more effort.  The Groovy 3 branch has an even older structure, but it is not taking fixes at that high of a rate.

What was the logic for what .java files stayed under "src/test/groovy" and which ones made it over to "src/test/java"?  I did prefer one unified file tree for tests.

Has there been any consideration for moving "src/spec/test" into "src/test/groovy"?  Or are those the tests that go with the language docs.


P.S.
In general, it would be nice to see 1) JUnit 5 for tests and 2) assertScript ''' test code ''' over bare groovy in test methods.  When working on parser or classgen changes, it is a bit of a barrier to ensure all the groovy code still compiles and executes successfully during an experiment.


________________________________
From: Paul King <pa...@asert.com.au>
Sent: Tuesday, April 15, 2025 3:14 AM
To: Groovy_Developers <de...@groovy.apache.org>
Subject: [EXT] PR to move tests to src/test/groovy

External Email: Use caution with links and attachments.

Hi folks,

We moved to the standard "src/main/groovy" (and src/main/java)
directories a long time ago. Likewise for src/test/groovy for all
modules except the core project. I created a PR for also moving the
core tests from "src/test" to the standard place:

https://urldefense.com/v3/__https://github.com/apache/groovy/pull/2194__;!!GFN0sa3rsbfR8OLyAw!biHaRXJ-g1UR119VMsgZeaa4MZ4dAL5mG_TaJVZwKoh7EbNFJmbxk-rlXsmt5ToogfBKt7JtXBNJrDhk5WZY9g$

It is a bit of a sideways move in the sense that it doesn't make any
difference to the overall dev experience, but I think it is useful to
be using the standard folders. Let me know if anyone objects to
applying the PR. It will be a little bit painful if you have a whole
bunch of tests you are about to commit/refactor, so I can wait until
convenient for everyone. You'll want to resync your IDE with the
Gradle changes once it has been committed (assuming we go ahead).

Cheers, Paul.

Re: [EXT] PR to move tests to src/test/groovy

[Paul King <pa...@asert.com.au>]

I should clarify, on Groovy 4, all tests are under src/test/groovy.
It's just under Groovy 5 that one file played up and hasn' t been
moved. I don't understand why just yet.

On Wed, Apr 16, 2025 at 8:49 PM Paul King <pa...@asert.com.au> wrote:
>
> Okay, I moved Groovy 4 src/test as well. I managed to move all but one
> of the files under src/test/java back into src/test/groovy.
> When I try to move groovy.NamedParameterHelper back I get:
>
> Execution failed for task ':compileTestGroovy'.
> > BUG! exception in phase 'semantic analysis' in source unit '/Users/paulk/Projects/groovy/src/test/groovy/groovy/NamedParameterTest.groovy' ConstructedClassWithPackage#setName should not be called
>
> This doesn't occur for Groovy 4_0_X.
>
> Other notes:
> * Our CI JVMs seem to lose locale info on JDK23+, so
> AppendableDgmMethodsTest checks against two possible output values.
> * For some reason,
> org.codehaus.groovy.benchmarks.vm5.b2394.ScriptLauncher needed a fully
> qualified reference to the Builder Java class in the same package.
> (Moving to src/test/java also fixes it without needing the extra
> qualification.) Adding the reference is an easy fix, but this could be
> an issue if it affects other projects.
> * On JVMs offering sealed classes,
> org.codehaus.groovy.util.ReferenceManagerTest#TestReference isn't
> liked because it isn't sealed. Since we don't have GROOVY-11292 on
> Groovy 4, I made the class final to sidestep the problem.
>
> Cheers, Paul.
>
> On Tue, Apr 15, 2025 at 11:29 PM Paul King <pa...@asert.com.au> wrote:
> >
> > ,
> > Comments inline.
> >
> > On Tue, Apr 15, 2025 at 11:10 PM Milles, Eric (TR Technology) via dev
> > <de...@groovy.apache.org> wrote:
> > >
> > > Since this has moved forward, would you be able to apply the same to Groovy 4?  With this in place, moving commits (with tests) to older release branches requires more effort.  The Groovy 3 branch has an even older structure, but it is not taking fixes at that high of a rate.
> >
> > I was planning to give this more time to percolate but I think it is
> > the right step forward. I'll see what I can do for Groovy 4. Might be
> > after some other things are off my urgent list.
> >
> > > What was the logic for what .java files stayed under "src/test/groovy" and which ones made it over to "src/test/java"?  I did prefer one unified file tree for tests.
> >
> > Many things broke and in some cases, moving the thing it was
> > complaining about from src/test/groovy to src/test/java fixed the
> > problem. I was guessing if it was generating stubs, they might have
> > been causing some of the problems. But, after fixing numerous other
> > things, it might be possible to move some of them back now.
> >
> > > Has there been any consideration for moving "src/spec/test" into "src/test/groovy"?  Or are those the tests that go with the language docs.
> >
> > src/spec/test are indeed the tests for the adoc files. A merger might
> > be possible but might make it harder to reference the right files and
> > harder to find them.
> >
> > >
> > > P.S.
> > > In general, it would be nice to see 1) JUnit 5 for tests and 2) assertScript ''' test code ''' over bare groovy in test methods.  When working on parser or classgen changes, it is a bit of a barrier to ensure all the groovy code still compiles and executes successfully during an experiment.
> >
> > There are definitely cases where assertScript is pretty essential, but
> > I also find there is a tradeoff between getting good IDE support and
> > having code in strings for assertScript.
> >
> > >
> > > ________________________________
> > > From: Paul King <pa...@asert.com.au>
> > > Sent: Tuesday, April 15, 2025 3:14 AM
> > > To: Groovy_Developers <de...@groovy.apache.org>
> > > Subject: [EXT] PR to move tests to src/test/groovy
> > >
> > > External Email: Use caution with links and attachments.
> > >
> > > Hi folks,
> > >
> > > We moved to the standard "src/main/groovy" (and src/main/java)
> > > directories a long time ago. Likewise for src/test/groovy for all
> > > modules except the core project. I created a PR for also moving the
> > > core tests from "src/test" to the standard place:
> > >
> > > https://urldefense.com/v3/__https://github.com/apache/groovy/pull/2194__;!!GFN0sa3rsbfR8OLyAw!biHaRXJ-g1UR119VMsgZeaa4MZ4dAL5mG_TaJVZwKoh7EbNFJmbxk-rlXsmt5ToogfBKt7JtXBNJrDhk5WZY9g$
> > >
> > > It is a bit of a sideways move in the sense that it doesn't make any
> > > difference to the overall dev experience, but I think it is useful to
> > > be using the standard folders. Let me know if anyone objects to
> > > applying the PR. It will be a little bit painful if you have a whole
> > > bunch of tests you are about to commit/refactor, so I can wait until
> > > convenient for everyone. You'll want to resync your IDE with the
> > > Gradle changes once it has been committed (assuming we go ahead).
> > >
> > > Cheers, Paul.

Re: [EXT] PR to move tests to src/test/groovy

[Paul King <pa...@asert.com.au>]

Okay, I moved Groovy 4 src/test as well. I managed to move all but one
of the files under src/test/java back into src/test/groovy.
When I try to move groovy.NamedParameterHelper back I get:

Execution failed for task ':compileTestGroovy'.
> BUG! exception in phase 'semantic analysis' in source unit '/Users/paulk/Projects/groovy/src/test/groovy/groovy/NamedParameterTest.groovy' ConstructedClassWithPackage#setName should not be called

This doesn't occur for Groovy 4_0_X.

Other notes:
* Our CI JVMs seem to lose locale info on JDK23+, so
AppendableDgmMethodsTest checks against two possible output values.
* For some reason,
org.codehaus.groovy.benchmarks.vm5.b2394.ScriptLauncher needed a fully
qualified reference to the Builder Java class in the same package.
(Moving to src/test/java also fixes it without needing the extra
qualification.) Adding the reference is an easy fix, but this could be
an issue if it affects other projects.
* On JVMs offering sealed classes,
org.codehaus.groovy.util.ReferenceManagerTest#TestReference isn't
liked because it isn't sealed. Since we don't have GROOVY-11292 on
Groovy 4, I made the class final to sidestep the problem.

Cheers, Paul.

On Tue, Apr 15, 2025 at 11:29 PM Paul King <pa...@asert.com.au> wrote:
>
> ,
> Comments inline.
>
> On Tue, Apr 15, 2025 at 11:10 PM Milles, Eric (TR Technology) via dev
> <de...@groovy.apache.org> wrote:
> >
> > Since this has moved forward, would you be able to apply the same to Groovy 4?  With this in place, moving commits (with tests) to older release branches requires more effort.  The Groovy 3 branch has an even older structure, but it is not taking fixes at that high of a rate.
>
> I was planning to give this more time to percolate but I think it is
> the right step forward. I'll see what I can do for Groovy 4. Might be
> after some other things are off my urgent list.
>
> > What was the logic for what .java files stayed under "src/test/groovy" and which ones made it over to "src/test/java"?  I did prefer one unified file tree for tests.
>
> Many things broke and in some cases, moving the thing it was
> complaining about from src/test/groovy to src/test/java fixed the
> problem. I was guessing if it was generating stubs, they might have
> been causing some of the problems. But, after fixing numerous other
> things, it might be possible to move some of them back now.
>
> > Has there been any consideration for moving "src/spec/test" into "src/test/groovy"?  Or are those the tests that go with the language docs.
>
> src/spec/test are indeed the tests for the adoc files. A merger might
> be possible but might make it harder to reference the right files and
> harder to find them.
>
> >
> > P.S.
> > In general, it would be nice to see 1) JUnit 5 for tests and 2) assertScript ''' test code ''' over bare groovy in test methods.  When working on parser or classgen changes, it is a bit of a barrier to ensure all the groovy code still compiles and executes successfully during an experiment.
>
> There are definitely cases where assertScript is pretty essential, but
> I also find there is a tradeoff between getting good IDE support and
> having code in strings for assertScript.
>
> >
> > ________________________________
> > From: Paul King <pa...@asert.com.au>
> > Sent: Tuesday, April 15, 2025 3:14 AM
> > To: Groovy_Developers <de...@groovy.apache.org>
> > Subject: [EXT] PR to move tests to src/test/groovy
> >
> > External Email: Use caution with links and attachments.
> >
> > Hi folks,
> >
> > We moved to the standard "src/main/groovy" (and src/main/java)
> > directories a long time ago. Likewise for src/test/groovy for all
> > modules except the core project. I created a PR for also moving the
> > core tests from "src/test" to the standard place:
> >
> > https://urldefense.com/v3/__https://github.com/apache/groovy/pull/2194__;!!GFN0sa3rsbfR8OLyAw!biHaRXJ-g1UR119VMsgZeaa4MZ4dAL5mG_TaJVZwKoh7EbNFJmbxk-rlXsmt5ToogfBKt7JtXBNJrDhk5WZY9g$
> >
> > It is a bit of a sideways move in the sense that it doesn't make any
> > difference to the overall dev experience, but I think it is useful to
> > be using the standard folders. Let me know if anyone objects to
> > applying the PR. It will be a little bit painful if you have a whole
> > bunch of tests you are about to commit/refactor, so I can wait until
> > convenient for everyone. You'll want to resync your IDE with the
> > Gradle changes once it has been committed (assuming we go ahead).
> >
> > Cheers, Paul.

Re: [EXT] PR to move tests to src/test/groovy

[Paul King <pa...@asert.com.au>]

,
Comments inline.

On Tue, Apr 15, 2025 at 11:10 PM Milles, Eric (TR Technology) via dev
<de...@groovy.apache.org> wrote:
>
> Since this has moved forward, would you be able to apply the same to Groovy 4?  With this in place, moving commits (with tests) to older release branches requires more effort.  The Groovy 3 branch has an even older structure, but it is not taking fixes at that high of a rate.

I was planning to give this more time to percolate but I think it is
the right step forward. I'll see what I can do for Groovy 4. Might be
after some other things are off my urgent list.

> What was the logic for what .java files stayed under "src/test/groovy" and which ones made it over to "src/test/java"?  I did prefer one unified file tree for tests.

Many things broke and in some cases, moving the thing it was
complaining about from src/test/groovy to src/test/java fixed the
problem. I was guessing if it was generating stubs, they might have
been causing some of the problems. But, after fixing numerous other
things, it might be possible to move some of them back now.

> Has there been any consideration for moving "src/spec/test" into "src/test/groovy"?  Or are those the tests that go with the language docs.

src/spec/test are indeed the tests for the adoc files. A merger might
be possible but might make it harder to reference the right files and
harder to find them.

>
> P.S.
> In general, it would be nice to see 1) JUnit 5 for tests and 2) assertScript ''' test code ''' over bare groovy in test methods.  When working on parser or classgen changes, it is a bit of a barrier to ensure all the groovy code still compiles and executes successfully during an experiment.

There are definitely cases where assertScript is pretty essential, but
I also find there is a tradeoff between getting good IDE support and
having code in strings for assertScript.

>
> ________________________________
> From: Paul King <pa...@asert.com.au>
> Sent: Tuesday, April 15, 2025 3:14 AM
> To: Groovy_Developers <de...@groovy.apache.org>
> Subject: [EXT] PR to move tests to src/test/groovy
>
> External Email: Use caution with links and attachments.
>
> Hi folks,
>
> We moved to the standard "src/main/groovy" (and src/main/java)
> directories a long time ago. Likewise for src/test/groovy for all
> modules except the core project. I created a PR for also moving the
> core tests from "src/test" to the standard place:
>
> https://urldefense.com/v3/__https://github.com/apache/groovy/pull/2194__;!!GFN0sa3rsbfR8OLyAw!biHaRXJ-g1UR119VMsgZeaa4MZ4dAL5mG_TaJVZwKoh7EbNFJmbxk-rlXsmt5ToogfBKt7JtXBNJrDhk5WZY9g$
>
> It is a bit of a sideways move in the sense that it doesn't make any
> difference to the overall dev experience, but I think it is useful to
> be using the standard folders. Let me know if anyone objects to
> applying the PR. It will be a little bit painful if you have a whole
> bunch of tests you are about to commit/refactor, so I can wait until
> convenient for everyone. You'll want to resync your IDE with the
> Gradle changes once it has been committed (assuming we go ahead).
>
> Cheers, Paul.