Test API for calling Mojos

[volodya-lombrozo]

We are using the next test code almost in all tests that are somehow connected with Mojos:

  Catalogs.INSTANCE.make(foreign)
            .add("foo.main")
            .set(AssembleMojo.ATTR_SCOPE, "compile")
            .set(AssembleMojo.ATTR_EO, src.toString());
        new Moja<>(ParseMojo.class)
            .with("targetDir", target.toFile())
            .with("foreign", foreign.toFile())
            .with("foreignFormat", "csv")
            .with("cache", temp.resolve("cache/parsed"))
            .execute();
        new Moja<>(OptimizeMojo.class)
            .with("targetDir", target.toFile())
            .with("foreign", foreign.toFile())
            .with("foreignFormat", "csv")
            .with("failOnError", false)
            .execute();

or

    private void discover(final Path temp) {
        final File target = temp.resolve("target").toFile();
        final File foreign = temp.resolve(DiscoverMojoTest.EO_FOREIGN).toFile();
        new Moja<>(ParseMojo.class)
            .with("targetDir", target)
            .with("foreign", foreign)
            .execute();
        new Moja<>(OptimizeMojo.class)
            .with("targetDir", target)
            .with("foreign", foreign)
            .execute();
        new Moja<>(DiscoverMojo.class)
            .with("targetDir", target)
            .with("foreign", foreign)
            .execute();
    }

In some test cases we even have already started to extract methods with that code, see discover() method above. When I write a new test, I quite often just copy that mess, add one more new Moja and continue (in most cases I even don't care about params for mojos). The problem with that code - is obvious duplication and verbosity, moreover it's hard to find the main idea of the test in all this mess. I think we have to invent some sort of Test API in order to decrease code duplication and simplify test code.

[yegor256]

@volodya-lombrozo I totally agree, but what would such an API look like?

[volodya-lombrozo]

@yegor256

It's just a sketch. But the Idea is to create an Object-Oriented API for each compilation step, then our tests will look like:

@Test
void testExample(){
  new Assemble(
      new CorrectProgram(),
     "trackOptimizationSteps=true" 
   ).execute();
  assertSomething();
}

instead of:

@Test
void testExample(){
        final Path src = temp.resolve("src");
        new Home(src).save(
            String.join(
                "\n",
                "+alias stdout org.eolang.io.stdout",
                "",
                "[x] > main\n  (stdout \"Hello!\" x).print\n"
            ),
            Paths.get("main.eo")
        );
        final Path target = temp.resolve("target");
        new Moja<>(RegisterMojo.class)
            .with("foreign", temp.resolve("eo-foreign.json").toFile())
            .with("foreignFormat", "json")
            .with("sourcesDir", src.toFile())
            .with("includeSources", new SetOf<>("**.eo"))
            .execute();
        new Moja<>(AssembleMojo.class)
            .with("outputDir", temp.resolve("out").toFile())
            .with("targetDir", target.toFile())
            .with("foreign", temp.resolve("eo-foreign.json").toFile())
            .with("foreignFormat", "json")
            .with("placed", temp.resolve("list").toFile())
            .with("cache", temp.resolve("cache/parsed"))
            .with("skipZeroVersions", true)
            .with("central", Central.EMPTY)
            .with("ignoreTransitive", true)
            .with(
                "objectionary",
                (Objectionary) input -> new InputOf(
                    "[] > sprintf\n"
                )
            )
            .execute();
      assertSomething();
}

In constructors we will have some sort of flexibility in adjusting different parameters for different cases. Also pay attention, that each step will use previous steps internally. It's important, because when I wanna test assmble phase I don't wanna see "how" it is performed and constructed.

[mximp]

@volodya-lombrozo We also need flexibility of choosing what mojos are executed. Ideally we want some kind of DSL which allows to specify steps (mojos) and their configurations.
In your example how would I provide a specific file for a property?

[mximp]

I believe some sort of predefined test configurations of steps might help here. It can use naming conventions for paths and file names to supply input and check output

[volodya-lombrozo]

@mximp

@volodya-lombrozo We also need flexibility of choosing what mojos are executed. Ideally we want some kind of DSL which allows to specify steps (mojos) and their configurations.
In your example how would I provide a specific file for a property?

We have OOP way to specify steps and their configurations, DSL - it's too much, as for me :)

new Assemble(
  new Register("foreign=eo-foreign.json"),
  "foreignFormat=json",
  "cache=/tmp/test"
).execute();

[volodya-lombrozo]

@mximp

I believe some sort of predefined test configurations of steps might help here. It can use naming conventions for paths and file names to supply input and check output

It's a dark magic which is hard to read and configure. But it's only my personal opinion. I can be wrong.

[mximp]

We have OOP way to specify steps and their configurations, DSL - it's too much, as for me :)

new Assemble(
  new Register("foreign=eo-foreign.json"),
  "foreignFormat=json",
  "cache=/tmp/test"
).execute();

@volodya-lombrozo you still have to specify all properties. Also it's not clear how to convert string /tmp/test to a Path object which is expected by AssembleMojo.cache.
In the end it will look mostly like what we have now.

[volodya-lombrozo]

We have OOP way to specify steps and their configurations, DSL - it's too much, as for me :)

new Assemble(
  new Register("foreign=eo-foreign.json"),
  "foreignFormat=json",
  "cache=/tmp/test"
).execute();

@volodya-lombrozo you still have to specify all properties. Also it's not clear how to convert string /tmp/test to a Path object which is expected by AssembleMojo.cache. In the end it will look mostly like what we have now.

@mximp Not really, In most cases we will use simple constructors without params (default properties will be set in each class inside constructors as default presets). As for different types of properties - I don't think it's a problem, we can use maps or a separate class for settings, it's more a matter of technique :)

[yegor256]

@mximp @volodya-lombrozo how about rewriting this test:

@Test
void testExample(){
        final Path src = temp.resolve("src");
        new Home(src).save(
            String.join(
                "\n",
                "+alias stdout org.eolang.io.stdout",
                "",
                "[x] > main\n  (stdout \"Hello!\" x).print\n"
            ),
            Paths.get("main.eo")
        );
        final Path target = temp.resolve("target");
        new Moja<>(RegisterMojo.class)
            .with("foreign", temp.resolve("eo-foreign.json").toFile())
            .with("foreignFormat", "json")
            .with("sourcesDir", src.toFile())
            .with("includeSources", new SetOf<>("**.eo"))
            .execute();
        new Moja<>(AssembleMojo.class)
            .with("outputDir", temp.resolve("out").toFile())
            .with("targetDir", target.toFile())
            .with("foreign", temp.resolve("eo-foreign.json").toFile())
            .with("foreignFormat", "json")
            .with("placed", temp.resolve("list").toFile())
            .with("cache", temp.resolve("cache/parsed"))
            .with("skipZeroVersions", true)
            .with("central", Central.EMPTY)
            .with("ignoreTransitive", true)
            .with(
                "objectionary",
                (Objectionary) input -> new InputOf(
                    "[] > sprintf\n"
                )
            )
            .execute();
      assertSomething();
}

as such:

@Test
void testExample(@TempDir temp){
  new FakeMaven()
    .home(temp.resolve("src"))
    .save(
      String.join(
        "\n",
        "+alias stdout org.eolang.io.stdout",
        "",
        "[x] > main\n  (stdout \"Hello!\" x).print\n"
      ),
      "main.eo"
    )
    .with("foreign", "./eo-foreign.json")
    .with("foreignFormat", "json")
    .with("sourcesDir", ".")
    .with("includeSources", "**.eo")
    .with("outputDir", "./out")
    .with("targetDir", ".")
    .with("placed", "./list")
    .with("cache", "./cache/parsed")
    .with("skipZeroVersions", true)
    .with("central", Central.EMPTY)
    .with("ignoreTransitive", true)
    .with(
        "objectionary",
        (Objectionary) input -> new InputOf(
            "[] > sprintf\n"
        )
    )
    .execute(RegisterMojo.class)
    .execute(AssembleMojo.class)
    .assertFileExists("./target/eo-foreign.json");
}

[volodya-lombrozo]

@yegor256 as for me, that code doesn't solve the original problem:

1. It isn't an OOP approach :)
2. The complexity is still here (you can even visually compare they between each other):
   2.1. Mess of parameters (of which I need only 1-2) for any test case. I more than sure, that people will just copy that mess between tests (at least I will do).
   2.2. The class FakeMaven or its interface is going to be hard to read (it definitely will contain tons of methods) - what about ISP in that case?
   2.3. I have to understand the "chain" of methods and return values for save, with, assert, execute and etc.
3. Quilice will complain that you don't use assertions. (Qulice doesn't treat a assertFileExists method as a correct assert)

[yegor256]

@volodya-lombrozo well... 2.2) we can have defaults, 2.3) this is fake-test class, it's OK to make it large, I believe, 3) let's do this instead:

assertThat(
  new FakeMaven()..., // returns a hash of all files
  allOf(
    hasKey("./target/eo-foreign.json"),
    hasEntry("./target/int.xmir", /* some XML matcher */)
  )
)

[volodya-lombrozo]

@yegor256 I'm still afraid of 2.2. Why should we consider the test classes as code that may violate all the principles? It's the same code as a production code, and, as for me, it's even more important code. But I can be wrong, of course, and probably I'm just exaggerating the problem.

[yegor256]

@volodya-lombrozo you are not wrong, but I don't see a mess. In most cases our code will look like this:

@Test
void testExample(@TempDir temp){
  assertThat(
    new FakeMaven()
      .home(temp.resolve("src"))
      .save(
        String.join(
          "\n",
          "+alias stdout org.eolang.io.stdout",
          "",
          "[x] > main\n  (stdout \"Hello!\" x).print\n"
        ),
        "main.eo"
      )
      .with("cache", "./cache/parsed")
      .execute(RegisterMojo.class)
      .execute(AssembleMojo.class),
    hasKey("./target/eo-foreign.json")
  );
}

I don't see a mess. Keep in mind, most .with() calls won't be needed, since we'll have defaults inside FakeMaven.

Even the save part may be omitted in most cases like this:

@Test
void testExample(@TempDir temp){
  assertThat(
    new FakeMaven()
      .home(temp.resolve("src"))
      .saveDefault()
      .with("cache", "./cache/parsed")
      .execute(RegisterMojo.class)
      .execute(AssembleMojo.class),
    hasKey("./target/eo-foreign.json")
  );
}

[volodya-lombrozo]

@yegor256 let's try. But I have one more question - can we create an objects abstractions for eo programs? In other words instead of that:

@Test
void testExample(@TempDir temp){
  assertThat(
    new FakeMaven()
      .home(temp.resolve("src"))
      .save(
        String.join(
          "\n",
          "+alias stdout org.eolang.io.stdout",
          "",
          "[x] > main\n  (stdout \"Hello!\" x).print\n"
        ),
        "main.eo"
      )
      .with("cache", "./cache/parsed")
      .execute(RegisterMojo.class)
      .execute(AssembleMojo.class),
    hasKey("./target/eo-foreign.json")
  );
}

can we use something like:

@Test
void testExample(@TempDir temp){
  assertThat(
    new FakeMaven()
      .home(temp.resolve("src"))
      .save(
        new SimpleEoProgram()
      )
      .with("cache", "./cache/parsed")
      .execute(RegisterMojo.class)
      .execute(AssembleMojo.class),
    hasKey("./target/eo-foreign.json")
  );
}

It:

1. increases readability (as for me)
2. decreases indents and lines of code
3. we can reuse such classes in a lot of other places, because, again, in most cases it isn't important to see the eo code, it's important to know that it is just "some" program which is correct (I'm not interesting in exact code).

[yegor256]

@volodya-lombrozo in general, I would suggest to use a simple rule: use scalar values first and THEN, when you see the necessity of an object, make an object. Java is not "object friendly" language. Each class, unfortunately, adds complexity to the code. Try to keep code simple first and OOP second :)

So, I'm not against this SimpleEoProgram class, but later, when we really see that it's needed. At the beginning, just a String will be enough, I'm sure.

[0pdd]

@volodya-lombrozo the puzzle #1479 is still not solved.

[0pdd]

@volodya-lombrozo 2 puzzles #1488, #1489 are still not solved; solved: #1479.

[0pdd]

@volodya-lombrozo the puzzle #1488 is still not solved; solved: #1479, #1489.

[0pdd]

@volodya-lombrozo all 3 puzzles are solved here: #1479, #1488, #1489.