feat(#2217): Apply FakeMaven in AssembleMojoTest #2221

volodya-lombrozo · 2023-07-07T14:14:40Z

I've applied FakeMaven for AssembleMojoTest and added checking prerequisites for RegisterMojo in order to show more meaningful message instead of just NullPointerException.

Closes: #2217

PR-Codex overview

This PR focuses on improving the AssembleMojoTest by using the FakeMaven class.

Detailed summary:

Refactored the tests in AssembleMojoTest to use the FakeMaven class
Added a new test case for assembling invalid eo programs
Updated the assertions in the test cases to use the FakeMaven result method

The following files were skipped due to too many changes: eo-maven-plugin/src/test/java/org/eolang/maven/AssembleMojoTest.java

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

…exuting it

…ilOnErrorFlag test

…leIfFailOnErrorFlagIsTrue

…Mojo

volodya-lombrozo · 2023-07-07T15:10:13Z

@maxonfjvipon Could you take a look, please?

eo-maven-plugin/src/main/java/org/eolang/maven/RegisterMojo.java

eo-maven-plugin/src/test/java/org/eolang/maven/AssembleMojoTest.java

…isite method

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java # eo-maven-plugin/src/test/java/org/eolang/maven/RegisterMojoTest.java

volodya-lombrozo · 2023-07-10T15:57:17Z

@maxonfjvipon Could you take a look one more time, please?

maxonfjvipon · 2023-07-10T16:20:11Z

eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java

+     * @return The same maven instance.
+     */
+    FakeMaven withoutDefaults() {
+        return new FakeMaven(this.workspace, this.params, this.attributes, this.current, false);


@volodya-lombrozo Do you think it's a good idea to return a new instance of FakeMaven here? It looks like in order to get FakeMaven without defaults you need to create first one with defaults and then call this method (like you did in test throwsExceptionInCaseSourceDirIsNotSet).
It's up to you but I would remove the method and uses constructor with 2 arguments in tests instead

@maxonfjvipon To be honest, it's a good suggestion. Thank you. However, I'm trying to hide that fact that FakeMaven is a configurable object:
https://www.yegor256.com/2016/04/19/object-must-not-be-configurable.html

Moreover, since FakeMaven implements some sort of the builder pattern, I believe that it's convenient to keep the corresponding style.

@volodya-lombrozo then I have no more questions, thanks.

maxonfjvipon · 2023-07-10T16:20:44Z

@volodya-lombrozo One comment from my side, see above

maxonfjvipon · 2023-07-10T16:41:52Z

@yegor256 could you please merge these changes?

volodya-lombrozo · 2023-07-11T08:02:37Z

@yegor256 Could you merge that changes please?

yegor256 · 2023-07-11T09:23:16Z

eo-maven-plugin/src/main/java/org/eolang/maven/RegisterMojo.java

@@ -108,6 +108,7 @@ public final class RegisterMojo extends SafeMojo {

    @Override
    public void exec() throws IOException {
+        this.verifyPrerequisite();


@volodya-lombrozo check this one: https://www.yegor256.com/2015/12/08/temporal-coupling-between-method-calls.html

volodya-lombrozo · 2023-07-11T10:19:03Z

@yegor256 I've inlined that method and added if-else statement. Could you have a look one more time, please?

volodya-lombrozo · 2023-07-12T08:27:01Z

@yegor256 Reminder

yegor256 · 2023-07-12T08:28:18Z

eo-maven-plugin/src/main/java/org/eolang/maven/RegisterMojo.java

-            if (this.scopedTojos().contains(name)) {
-                Logger.debug(this, "EO source %s already registered", name);
-                continue;
+        if (this.sourcesDir == null) {


@volodya-lombrozo check this: https://www.yegor256.com/2015/01/21/if-then-throw-else.html

@yegor256 Done. Thanks

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java

volodya-lombrozo · 2023-07-12T09:13:33Z

@yegor256 Could you merge that changes, please?

yegor256 · 2023-07-12T09:14:38Z

@rultor merge

rultor · 2023-07-12T09:17:09Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2023-07-12T09:27:01Z

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 10min)

volodya-lombrozo added 5 commits July 7, 2023 16:04

feat(objectionary#2217): check prerequisites for RegisterMojo before …

83a13ce

…exuting it

feat(objectionary#2217): refactor AssembleMojo.assemblesTogether test

1492165

feat(objectionary#2217): refactor AssembleMojo.assemblesNotFailWithFa…

7a68589

…ilOnErrorFlag test

feat(objectionary#2217): add one more test AssembleMojo.doesNotAssemb…

2637b87

…leIfFailOnErrorFlagIsTrue

feat(objectionary#2217): remove the puzzle for the 2217 issue

e42635e

volodya-lombrozo force-pushed the 2217_fake_maven_assembly branch from e1c5bfb to fc73673 Compare July 7, 2023 14:34

feat(objectionary#2217): simplify checking prerequisites for Register…

f7a4a18

…Mojo

volodya-lombrozo force-pushed the 2217_fake_maven_assembly branch from fc73673 to f7a4a18 Compare July 7, 2023 14:43

volodya-lombrozo requested a review from maxonfjvipon July 7, 2023 15:10

maxonfjvipon suggested changes Jul 10, 2023

View reviewed changes

volodya-lombrozo added 3 commits July 10, 2023 18:34

feat(objectionary#2217): Add test case for RegisterMojo.verifyPrerequ…

ed2def3

…isite method

feat(objectionary#2217): fix grammar mistakes

9e34b2d

Merge branch 'master' into 2217_fake_maven_assembly

8e6c572

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java # eo-maven-plugin/src/test/java/org/eolang/maven/RegisterMojoTest.java

volodya-lombrozo requested a review from maxonfjvipon July 10, 2023 15:57

maxonfjvipon suggested changes Jul 10, 2023

View reviewed changes

maxonfjvipon approved these changes Jul 10, 2023

View reviewed changes

yegor256 reviewed Jul 11, 2023

View reviewed changes

volodya-lombrozo added 2 commits July 11, 2023 12:35

feat(objectionary#2217): inline checking method

f1bd609

feat(objectionary#2217): fix qulice suggestions

a523c0c

yegor256 reviewed Jul 12, 2023

View reviewed changes

volodya-lombrozo added 3 commits July 12, 2023 11:38

feat(objectionary#2217): remove the 'else' block

4c6858e

Merge branch 'master' into 2217_fake_maven_assembly

2812e18

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/FakeMaven.java

feat(objectionary#2217): remove the old attributes variables and params

9f43509

volodya-lombrozo requested a review from yegor256 July 12, 2023 09:13

rultor merged commit b2f082d into objectionary:master Jul 12, 2023
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#2217): Apply FakeMaven in AssembleMojoTest #2221

feat(#2217): Apply FakeMaven in AssembleMojoTest #2221

volodya-lombrozo commented Jul 7, 2023 •

edited by pr-codex bot

Loading

volodya-lombrozo commented Jul 7, 2023

volodya-lombrozo commented Jul 10, 2023

maxonfjvipon Jul 10, 2023 •

edited

Loading

volodya-lombrozo Jul 10, 2023

maxonfjvipon Jul 10, 2023

maxonfjvipon commented Jul 10, 2023

maxonfjvipon commented Jul 10, 2023

volodya-lombrozo commented Jul 11, 2023

yegor256 Jul 11, 2023

volodya-lombrozo commented Jul 11, 2023

volodya-lombrozo commented Jul 12, 2023

yegor256 Jul 12, 2023

volodya-lombrozo Jul 12, 2023

volodya-lombrozo commented Jul 12, 2023

yegor256 commented Jul 12, 2023

rultor commented Jul 12, 2023

rultor commented Jul 12, 2023

feat(#2217): Apply FakeMaven in AssembleMojoTest #2221

feat(#2217): Apply FakeMaven in AssembleMojoTest #2221

Conversation

volodya-lombrozo commented Jul 7, 2023 • edited by pr-codex bot Loading

PR-Codex overview

Detailed summary:

volodya-lombrozo commented Jul 7, 2023

volodya-lombrozo commented Jul 10, 2023

maxonfjvipon Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

volodya-lombrozo Jul 10, 2023

Choose a reason for hiding this comment

maxonfjvipon Jul 10, 2023

Choose a reason for hiding this comment

maxonfjvipon commented Jul 10, 2023

maxonfjvipon commented Jul 10, 2023

volodya-lombrozo commented Jul 11, 2023

yegor256 Jul 11, 2023

Choose a reason for hiding this comment

volodya-lombrozo commented Jul 11, 2023

volodya-lombrozo commented Jul 12, 2023

yegor256 Jul 12, 2023

Choose a reason for hiding this comment

volodya-lombrozo Jul 12, 2023

Choose a reason for hiding this comment

volodya-lombrozo commented Jul 12, 2023

yegor256 commented Jul 12, 2023

rultor commented Jul 12, 2023

rultor commented Jul 12, 2023

volodya-lombrozo commented Jul 7, 2023 •

edited by pr-codex bot

Loading

maxonfjvipon Jul 10, 2023 •

edited

Loading