Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a maven-polyglot test using takari yaml extension #158

Closed
wants to merge 2 commits into from

Conversation

McFoggy
Copy link

@McFoggy McFoggy commented Sep 30, 2020

This is a simple contribution with an example of a test on a maven project using a core extension.
In this case, I used the polyglot-maven takari extension and more precisely the yaml polyglot one.

As the introduced tests demo that ITF does not support core extension manipulating the POM (as Object Model), I also introduced a profile for the introduced "Failing" test.

Normal commands will not be impacted and the failing tests are exlcluded from the normal build.
They have to be explicitly run by using the profile failing on the example project using mvn -Pfailing verify

…l extension

addition of a profile to demo failing test, use `mvn -P failing verify` on the example project to run failing tests
@khmarbaise
Copy link
Owner

First many thanks for your contribution.

But I have some questions:

  • This part ..demo that ITF does not support core extension manipulating the POM.. I simply don't understand what you mean by that? Can you elaborate more in detail your idea/intention?

I have seen that you are changing the itf-examples/pom.xml file like this:

  <execution>
            <id>standard</id>

This means you are adding a supplemental execution id which is already handled by Maven without supplemental configuration. So this does not really makes sense. If you take a look while building the whole project you can observe it like this:

[INFO] Installing /.../maven-it-extension/itf-examples/pom.xml to /.../maven-it-extension/itf-examples/target/itf-repo/com/soebes/itf/jupiter/extension/itf-examples/0.10.0-SNAPSHOT/itf-examples-0.10.0-SNAPSHOT.pom
[INFO] Installing /.../maven-it-extension/itf-examples/target/itf-examples-0.10.0-SNAPSHOT.jar to /.../itf-examples/target/itf-repo/com/soebes/itf/jupiter/extension/itf-examples/0.10.0-SNAPSHOT/itf-examples-0.10.0-SNAPSHOT.jar
[INFO] Installing /.../maven-it-extension/itf-examples/target/itf-examples-0.10.0-SNAPSHOT-tests.jar to /.../maven-it-extension/itf-examples/target/itf-repo/com/soebes/itf/jupiter/extension/itf-examples/0.10.0-SNAPSHOT/itf-examples-0.10.0-SNAPSHOT-tests.jar
[INFO] 
[INFO] --- maven-failsafe-plugin:3.0.0-M4:integration-test (default) @ itf-examples ---
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.soebes.itf.examples.EARIT
[INFO] Running com.soebes.itf.examples.goals.MetaAnnotationGoalIT
[INFO] Running com.soebes.itf.examples.options.OptionsIT
[INFO] Running com.soebes.itf.examples.MavenIntegrationExampleNestedWithoutNoneNestedTestGlobalRepoIT
[INFO] Running com.soebes.itf.examples.options.OptionsOnClassIT

The (default) is the id which is defined by default (as the name implies) which is fine. I don't see any need to change that. (Following convention over configuration paradigm).

Furthermore you are changing the failsafe plugin and trying to cut it from the life cycle which results in not executing any kind of IT...which is not a good idea. This would mean not to execute any IT in the itf-example's project which I don't want to. In itf-example all IT's should run always...

  <artifactId>maven-failsafe-plugin</artifactId>
            <executions>
              <execution>
                <id>standard</id>
                <phase>none</phase>
              </execution>

I'm not sure what the intention is to change the itf-example/pom.xml file and adding some kind of profiles. It looks like you are trying to control the execution of your test project in itf-examples/src/test/resources-its/com/soebes/itf/examples/polyglot/PolyglotIT/yaml/pom.yaml but this should be controlled by the Java code in PolyglotIT and not via the pom file of the itf-example project.

You can define profiles within an integration test via the @MavenProfile annotation (see users guide) ... I could think about it like this:

@MavenJupiterExtension
public class PolyglotIT {

  @MavenTest
  void yaml_ok(MavenExecutionResult result) {
    assertThat(result).isSuccessful();
  }
  @MavenTest
  @MavenProfile("failing")
  void yaml_fail(MavenExecutionResult result) {
    assertThat(result).isFailure();
  }

}

Unfortunately I will not accept a change which is changing itf-examples/pom.xml (except for very special reasons) because all integration tests should handle that within the java (for example in PolyglotIT code and not in the pom file...

@McFoggy
Copy link
Author

McFoggy commented Oct 5, 2020

The (default) is the id which is defined by default (as the name implies) which is fine

It was my intention, I don't know why I used standard instead of default.
The proposal was to not disturb existing tests with a failing one that cannot pass without a modification of ITF.
That's why I wanted to exclude by default the test category Failing and activate it only in a specific profile.

AFAIK, to do the above, the default configuration of failsafe needs to be modified ; isn't it?

@khmarbaise
Copy link
Owner

Hi,

The proposal was to not disturb existing tests with a failing one that cannot pass without a modification of ITF.
That's why I wanted to exclude by default the test category Failing and activate it only in a specific profile.
As I wrote the pom file should never needed to be changed. The expected failing or not expected failing of an integration test should be done in the java code and not in the pom file..

The integration tests will tell you: This test has failed which means your plugin/extension etc. which is under test does not behave as expected. (expressed by the test.)
This is the same as writing a unit tests. The test describes the expected behaviour.

@McFoggy
Copy link
Author

McFoggy commented Oct 5, 2020

I know that the integration will fail ; and it is as it is the purpose to demo that ITF needs a modification to handle correctly core extensions and POM modifications on load.

But if I PR an IT test that fails your build and break the CI I think you will not accept it neither. That's why I wanted to isolate the execution of this failing test in a different profile.

If you have/know a better way of providing a failing test, please tell me I'll adapt the PR.

@McFoggy
Copy link
Author

McFoggy commented Oct 5, 2020

In my opinion it is not expected to write the IT test with the opposite meaning ; ie consider the execution failure as an IT test success.
But if you want it like that ; I can reverse the test and then remove the 'failing' profile.

@McFoggy
Copy link
Author

McFoggy commented Oct 5, 2020

closing in favor of the simplified (but failing the CI) #165

@McFoggy McFoggy closed this Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants