feat: support test source dirs in the plugin #68

MartinWitt · 2020-04-17T11:13:47Z

should do the trick, with adding a new parameter for test src. Sadly I need to fix maven issues before i can test it, because a dependency cant be resolved. If somebody wants to test it, it could be faster.

For the record the following fails:

<dependency>
      <groupId>fr.inria.gforge.spoon</groupId>
      <artifactId>processors</artifactId>
      <version>1.0-SNAPSHOT</version>
      <scope>test</scope>
  </dependency>

MartinWitt · 2020-04-17T12:17:33Z

should be working now.

vmassol · 2020-04-17T12:35:54Z

src/test/java/fr/inria/gforge/spoon/mojo/SpoonMojoTest.java

+		assertThat(contentSource).doesNotExist();
+
+		final File resultFileProcessor = new File(basedir, "target/spoon-maven-plugin/spoon-nb-statement.txt");
+		assertThat(resultFileProcessor).exists();


I think this test will always pass. Because you have an App.java class and thus the resultFileProcessor will always exist. Am I wrong?

from my understanding the test will always fail, because we need the processors from the "/resources/processors/"

Note: on my side I didn't try the test I wrote because the maven build was failing.

The processor is at

spoon-maven-plugin/src/test/java/fr/inria/gforge/spoon/mojo/CountStatementProcessor.java

Line 10 in 4bd92ae

public class CountStatementProcessor extends AbstractProcessor<CtStatement> {

, ,no? So it should be there in the CP.

What I meant is that you're testing that the check worked by verifying that the output file exists. But this file will be created by the processor running on the main source, no?

Side note: this is why I didn't have any App.java class in my test.

All this is speculation since I didn't run the maven build :)

having only test classes is a way better test, good point. Changed it.

MartinWitt · 2020-04-17T13:11:04Z

now it should only test with the test source file.
You can fix the build with change your directory to /test/projects/processors and running mvn clean install -"Dmaven.compiler.source"=1.8 -"Dmaven.compiler.target"=1.8. Doing that you can already use the plugin and dont have to wait for merge, or try the prebuild from me.
spoon-maven-plugin-3.4-SNAPSHOT.zip

MartinWitt · 2020-04-17T13:22:09Z

Can i remove the edit in the original issue and see this as solved/ready for review/merge?

vmassol · 2020-04-17T13:25:27Z

Can i remove the edit in the original issue and see this as solved/ready for review/merge?

Is that a question for me? :) If so, not sure what you mean.

MartinWitt · 2020-04-17T13:26:46Z

Yes was a question for you because i added

Edit: nvm vmassol found something that needs to be fixed sry

Afterwards, because you found some open problem

vmassol · 2020-04-17T13:39:45Z

@MartinWitt I've closed #68 if that's what you meant. Thanks!

monperrus

Thanks a lot @MartinWitt you first contribution to spoon-maven-plugin will be very useful.

See comments, esp to improve the test of thee new feature.

src/main/java/fr/inria/gforge/spoon/SpoonMojoGenerate.java

monperrus · 2020-04-18T04:02:59Z

src/test/projects/hello-world-with-test/pom.xml

+      <plugin>
+        <groupId>fr.inria.gforge.spoon</groupId>
+        <artifactId>spoon-maven-plugin</artifactId>
+        <configuration>


the processor should be configured in full classpath

I dont understand what you mean sry. Could you explain it?

the processor should be configured in full classpath

@monperrus BTW could you explain why you've removed fullclasspath by default? It seemed like it would have better chance to work by default than having to force a configuration parameter, no? What am I missing? And yes I'm sure there are good reasons I don't know, I haven't followed this discussion/decision. I'm just curious to know the reasons. Thx!

I dont understand what you mean sry. Could you explain it?

I think @monperrus means using <noclasspath>false</noclasspath> or something like this but I'm not sure either.

@monperrus BTW could you explain why you've removed fullclasspath by default?

It was removed from spoon-core by default, we did not make any explicit decision for spoon-maven-plugin, it inherits the default of spoon-core.

Why we set the default to NOCLASSPATH in spoon-core? Because it mawimizes the chance that a newcomer gets something interesting out of spoon without reading the doc and without setting a correct full classpath.

src/test/projects/hello-world-with-test/test/main/java/fr/inira/gforge/spoon/AppTest.java

monperrus · 2020-04-21T04:46:43Z

Now, we need to expand .travis.xml to check that spoon-maven-plugin works on hello-world-with-test/pom.xml` from the command-line. Thanks!

MartinWitt · 2020-04-21T09:07:31Z

Still need to lookup, how to specify classpath entries, for using <noClasspath>false</noClasspath>. Rest of review should be resolved

monperrus · 2020-04-22T09:26:29Z

Many many thanks @MartinWitt for the progress here.

Still need to lookup, how to specify classpath entries, for using false.

See #69

@vmassol Does this meet your use case?

MartinWitt · 2020-05-01T11:17:01Z

README.md

@@ -160,6 +160,7 @@ Maps and objects are created like this:
 ## How to change source and output folder?

 You can specify at spoon its input and output directories with, respectively, `srcFolder` and `outFolder` tags.


still needs rewrite.

MartinWitt · 2020-05-01T11:22:57Z

The testConfigurationOfTheCustomInputFolder shows, that removing option removes the ability to check only some custom folder. Readd the sourceFolder setting or lose the ability?
Edit: @monperrus wrote above, the feature is rarely used.

This reverts commit 5bb6a3c.

monperrus · 2020-05-04T17:56:34Z

OK for me to remove this feature.

MartinWitt · 2020-05-07T10:21:12Z

All todos are resolved now.

monperrus · 2020-05-07T13:14:21Z

Thanks a lot @MartinWitt , that was long but very important work to be done.

monperrus · 2020-05-07T13:14:32Z

Thanks @vmassol for all your inputs.

vmassol · 2020-05-07T13:25:02Z

Yippee; thanks a lot guys, you're great :) Big up to @MartinWitt

vmassol · 2020-05-13T15:42:52Z

@monperrus @MartinWitt Any idea when a spoon version containing this will be available? :) Thanks!

MartinWitt · 2020-05-13T23:27:20Z

Sadly releases are out of my scope, you have to wait for a maintainer(?) for a release.
But i see no reason to not release a new version 3.4 after this breaking change.

monperrus · 2020-05-14T05:26:03Z

I'll do the release. @vmassol do you confirm that everything works as expected on your setup?

vmassol · 2020-05-14T07:18:55Z

bq. @vmassol do you confirm that everything works as expected on your setup?

I was actually waiting for the release to test :) Problem is that I probably won't have the time today to test it (I'm in meeting all day). Now the issue represents my need for sure and you have tests for it I guess so it should be fine. If you prefer me to test it, then I could do that Friday.

Thx

monperrus · 2020-05-14T09:48:49Z

If you prefer me to test it, then I could do that Friday.

Yes, that would be better. We're not in a rush anyway.

vmassol · 2020-05-14T10:53:09Z

Yes, that would be better. We're not in a rush anyway.

Actually I won't have the time because of the release. Maybe next week, but not sure yet.

The "rush" was because I wanted to have this before the 12.4RC1 release of XWiki but from what I see it's going to be too late (I don't like to have an issue with commits and not closed for a release). But it's ok, it'll go in the next release in one month. It just means the spoon check is not active on xwiki FTM so more violations can be added any day. Definitely not critical but nice to have ASAP.

My point is that you should not wait for me. You need to have your own tests so that you're sure that it works.

But I'll be happy to help if I can, but will take some time.

Thx

monperrus · 2020-05-15T05:19:25Z

No worries, we move forward.

Release 3.4 done!

@MartinWitt would you prepare a changelog to be added to https://github.com/SpoonLabs/spoon-maven-plugin/releases/tag/spoon-maven-plugin-3.4?

Thanks!

vmassol · 2020-05-15T07:26:05Z

Thanks @monperrus!

vmassol · 2020-05-15T07:28:38Z

@MartinWitt I think this comment needs to be modified since you've removed the testFolder config properties:

spoon-maven-plugin/src/main/java/fr/inria/gforge/spoon/SpoonMojoGenerate.java

Line 37 in 4f6de46

    
           // Use the TEST scope so that both compile and test dependency artifacts are put in the Spoon classloader

Thanks

vmassol · 2020-05-15T08:47:08Z

@MartinWitt @monperrus I think I've found a regression in v3.4, see INRIA/spoon#3366

EDIT: Confirmed that it's a regression. I've re-run our build with v3.3 and it passed.

MartinWitt · 2020-05-15T10:21:52Z

@monperrus changelog comes after fixing the regression if that is okay for you?
@vmassol looking into the comment, after fixing the new bug, if that is okay for you?

vmassol · 2020-05-15T10:37:29Z

Sure, perfectly fine. Thx a lot.

monperrus · 2020-05-15T12:10:45Z

@monperrus changelog comes after fixing the regression if that is okay for you?

yes! we will release 3.4.1 and changelog it directly

add test dirs to the plugin

0d9bb0f

MartinWitt mentioned this pull request Apr 17, 2020

spoon-maven-plugin: add support for test source directories INRIA/spoon#3326

Closed

MartinWitt added 4 commits April 17, 2020 13:56

add testcase and comment from @vmassol

de78fce

remove quotes

83282e6

fix testcase and change checkmojo scope

af96c25

fix name matching error in testcase

99dc329

add testcase for check mojo

2c30437

MartinWitt marked this pull request as ready for review April 17, 2020 12:27

MartinWitt added 2 commits April 17, 2020 14:30

add check for file

7d50631

update readme

aacac48

vmassol reviewed Apr 17, 2020

View reviewed changes

MartinWitt added 2 commits April 17, 2020 14:38

remove check, because processor dependency is currently complex

7fee932

fix testcase

8c29b2d

monperrus reviewed Apr 18, 2020

View reviewed changes

add junit reference to test code

65992e4

MartinWitt added 3 commits April 21, 2020 10:41

remove method according to review

bc99b94

fix ci

c6430ac

add dependency

8d3594c

monperrus mentioned this pull request Apr 22, 2020

feat(spoon-maven-plugin): offer ability to use compile test source roots INRIA/spoon#3327

Closed

MartinWitt changed the title ~~add test dir to the plugin~~ feat: add test dir to the plugin Apr 22, 2020

change noClasspath to false ( => use classpath)

2c43e68

MartinWitt commented May 1, 2020

View reviewed changes

MartinWitt added 4 commits May 1, 2020 13:27

update testcase to new behaivor

404d005

remove hardcoded version

5bb6a3c

remove check and add comment

da693c5

Revert "remove hardcoded version"

6a01aeb

This reverts commit 5bb6a3c.

MartinWitt added 3 commits May 7, 2020 12:04

remove old testcases, testing an error, that no longer exists

cf4b2f2

remove todo. because resolved

b3a1156

update readme

ebd7f71

MartinWitt requested a review from monperrus May 7, 2020 10:21

monperrus changed the title ~~feat: add test dir to the plugin~~ feat: support test source dirs in the plugin May 7, 2020

monperrus merged commit 4b14e59 into SpoonLabs:master May 7, 2020

		@@ -160,6 +160,7 @@ Maps and objects are created like this:
		## How to change source and output folder?

		You can specify at spoon its input and output directories with, respectively, `srcFolder` and `outFolder` tags.

feat: support test source dirs in the plugin #68

feat: support test source dirs in the plugin #68

Conversation

MartinWitt commented Apr 17, 2020 • edited by monperrus Loading

MartinWitt commented Apr 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinWitt commented Apr 17, 2020 • edited Loading

MartinWitt commented Apr 17, 2020

vmassol commented Apr 17, 2020

MartinWitt commented Apr 17, 2020

vmassol commented Apr 17, 2020

monperrus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monperrus commented Apr 21, 2020

MartinWitt commented Apr 21, 2020 • edited Loading

monperrus commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinWitt commented May 1, 2020 • edited Loading

monperrus commented May 4, 2020

MartinWitt commented May 7, 2020

monperrus commented May 7, 2020

monperrus commented May 7, 2020

vmassol commented May 7, 2020

vmassol commented May 13, 2020

MartinWitt commented May 13, 2020

monperrus commented May 14, 2020 via email

vmassol commented May 14, 2020

monperrus commented May 14, 2020 via email

vmassol commented May 14, 2020

monperrus commented May 15, 2020

vmassol commented May 15, 2020

vmassol commented May 15, 2020

vmassol commented May 15, 2020 • edited Loading

MartinWitt commented May 15, 2020

vmassol commented May 15, 2020

monperrus commented May 15, 2020

MartinWitt commented Apr 17, 2020 •

edited by monperrus

Loading

MartinWitt commented Apr 17, 2020 •

edited

Loading

MartinWitt commented Apr 21, 2020 •

edited

Loading

MartinWitt commented May 1, 2020 •

edited

Loading

vmassol commented May 15, 2020 •

edited

Loading