Xray-App / xray-junit-extensions

Enhanced JUnit 5 integration with Xray Test Management for Jira
Eclipse Public License 2.0
16 stars 8 forks source link

Ability do define description at runtime #54

Closed mk868 closed 2 months ago

mk868 commented 3 months ago

Hello,

In my test code, I have a lot of descriptions most of which are repetitive and could be generated from the Java code. I would expect something similar to the @DisplayNameGeneration from JUnit5 - the annotation that specifies what class will create a description string:

// imports

@Target({ElementType.TYPE, ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
public @interface XrayTestGenerator {

    Class<? extends XrayGenerator > value();

}
// imports
import org.junit.jupiter.api.TestInfo;

interface XrayGenerator {

  default String generateDescription(TestInfo testInfo) {
    return null;
  }

}

This allows me to create an implementation of the XrayGenerator interface and define a description based on other test method annotations.

What do you think about it?

mk868 commented 3 months ago

My use case:

I would like to create such a test:

@Test
@XrayTest(key = "XXXX-1000", summary = "Simple permissions test AAA, BBB, CCC")
@CreateUserWithPermissions({Perm.AAA, Perm.BBB, Perm.CCC}) // custom annotation - specifies the permissions of the user to be created before the test
@XrayTestGenerator(PermissionsDescriptionGenerator.class)
@ExpectedPermissionTestResult("""
      * button A visible
      * button B invisible
      """) // custom annotation for expected behavior
void test1() {
  // code here...
}

CreateUserWithPermissions.java:

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface CreateUserWithPermissions {

  String[] value();
}

ExpectedPermissionTestResult.java:

@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
public @interface ExpectedPermissionTestResult {

  String value();
}

PermissionsDescriptionGenerator.java:

public class PermissionsDescriptionGenerator implements XrayGenerator { // custom implementation of XrayGenerator

  @Override
  public String generateDescription(TestInfo testInfo) {
    var permissions = testInfo.getTestMethod()
        .map(m -> m.getAnnotation(CreateUserWithPermissions.class))
        .map(CreateUserWithPermissions::value)
        .map(List::of)
        .orElse(List.of());

    var result = new StringBuilder();
    result.append("""
        Some description...
        Testing with user with permissions:
        """);

    permissions.forEach(perm -> {
      result.append("* ").append(perm).append("\n");
    });

    result.append("Expected result:\n");

    testInfo.getTestMethod()
        .map(m -> m.getAnnotation(ExpectedPermissionTestResult.class))
        .map(ExpectedPermissionTestResult::value)
        .ifPresent(result::append);

    return result.toString();
  }
}

this code will create description:

Some description...
Testing with user with permissions:
* AAA
* BBB
* CCC
Expected result:
* button A visible
* button B invisible

Such automation of the creation of descriptions will simplify their maintenance which becomes problematic with dozens of such similar tests

mk868 commented 3 months ago

This proposal may also help with #33 - we can generate custom description for each parameterized test...

Hmm, we can even add the ability to generate a custom summary/key like this:

interface XrayGenerator {

  default String generateKey(TestInfo testInfo) {
    return null;
  }

  default String generateSummary(TestInfo testInfo) {
    return null;
  }

  default String generateDescription(TestInfo testInfo) {
    return null;
  }

}
bitcoder commented 3 months ago

Hi @mk868 , thanks for your suggestion. So , and to make sure I got it right, the use case is that:

1) is my previous understanding accurate? 2) wouldn't in your case make more sense to just have one parameterized/data-driven JUnit5 test, instead of multiple test cases? Just wanted to understand your rationale for having multiple similar tests instead of a single data-driven/parameterized one. 3) In the end, and to be a bit pragrammatic, your suggestion would be to add exactly what to this project? The XrayTestGenerator interface?

mk868 commented 3 months ago

Hi,

  1. Yes, it's accurate
  2. There are two points:

    • The requirement in our test system is to explicitly link the test in Jira to the test run in the code - this decision was made at the beginning of the project many years ago. Each test method must be annotated with a Jira key, in the past we used a custom annotation + custom JUnit extension to synchronize with Jira, last year we have switched to @XrayTest annotation + xray-maven-plugin.
    • With data-driven/parameterized I'm not able to specify Jira Test key for each test execution, it's not possible to do something like that:
      
      // for this example I would like the results to be synced for 4 tests in Jira: ABC-100, ABC-101, ABC-102, ABC-103

    private static Stream dataSource() { return Stream.of( // first argument: Jira key // second argument: actual test parameter of("ABC-100", "PRODUCT_TABLE"), of("ABC-101", "ITEM_TABLE"), of("ABC-102", "USERS_TABLE"), of("ABC-103", "PRODUCT_TABLE") ); }

    @XrayTest @ParameterizedTest @MethodSource("dataSource") void sortTableTest(String _key, String tableName) { // _key param is ignored in the test code // tableName is used in the code

    // test code... }

  3. This is a subject to think about, I currently see 3 possibilities:
    • Create XrayGenerator interface + XrayTestGenerator annotation
    • Create XrayGenerator interface + enrich XrayTest annotation with Class<? extends XrayGenerator> dataGenerator() default XrayGenerator.NOP; field
    • Enabling a simple extension from app.getxray.xray.junit.customjunitxml.EnhancedLegacyXmlReportGeneratingListener to be able to inject key/summary/description values. Then I can implement the "XrayTestGenerator" logic myself.

Or do you have any other suggestions?

bitcoder commented 3 months ago
  1. ok
  2. we need to analyze the pros and cons (feedback is welcome!); the idea would be to avoid redundancy of features and trying to have this flexible at the same time.

I was looking at JUnit's 5 @DisplayNameGeneration annotation and wondering if that couldn't address this need perhaps with out-of-the-box JUnit5 features and the fact that this extension also looks at the displayName?

mk868 commented 3 months ago

From what I see @DisplayNameGeneration is limited in providing extra data (key/description).

Just thinking:

We can also prepare an abstraction of reading id/key/summary/description values that is used in XmlReportWriter#writeTestcase:

public interface XrayTestInfoReader {

    Optional<String> getId(TestIdentifier testIdentifier);

    Optional<String> getKey(TestIdentifier testIdentifier);

    Optional<String> getSummary(TestIdentifier testIdentifier);

    Optional<String> getDescription(TestIdentifier testIdentifier);
}

The default implementation (I simply copied that from the XmlReportWriter#writeTestcase) would look like this:

import app.getxray.xray.junit.customjunitxml.annotations.XrayTest;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.TestFactory;
import org.junit.platform.commons.support.AnnotationSupport;
import org.junit.platform.engine.TestSource;
import org.junit.platform.engine.support.descriptor.MethodSource;
import org.junit.platform.launcher.TestIdentifier;

import java.lang.reflect.Method;
import java.util.Optional;
import java.util.stream.Stream;

import static java.util.function.Predicate.not;

public class DefaultXrayTestInfoReader implements XrayTestInfoReader {

    @Override
    public Optional<String> getId(TestIdentifier testIdentifier) {
        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        Optional<XrayTest> xrayTest = AnnotationSupport.findAnnotation(testMethod, XrayTest.class);

        return xrayTest
                .map(XrayTest::id)
                .filter(not(String::isEmpty));
    }

    @Override
    public Optional<String> getKey(TestIdentifier testIdentifier) {
        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        Optional<XrayTest> xrayTest = AnnotationSupport.findAnnotation(testMethod, XrayTest.class);

        return xrayTest
                .map(XrayTest::key)
                .filter(not(String::isEmpty));
    }

    @Override
    public Optional<String> getSummary(TestIdentifier testIdentifier) {
        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        final Class testClass = ((MethodSource) testSource.get()).getJavaClass();
        Optional<XrayTest> xrayTest = AnnotationSupport.findAnnotation(testMethod, XrayTest.class);

        Optional<String> test_summary = xrayTest
                .map(XrayTest::summary)
                .filter(not(String::isEmpty));
        if (test_summary.isPresent()) {
            return test_summary;
        }

        Optional<DisplayName> displayName = AnnotationSupport.findAnnotation(testMethod, DisplayName.class);
        if (displayName.isPresent()) {
            return Optional.of(displayName.get().value());
        }

        Optional<TestFactory> dynamicTest = AnnotationSupport.findAnnotation(testMethod, TestFactory.class);
        Optional<DisplayNameGeneration> displayNameGenerator = AnnotationSupport.findAnnotation(testClass, DisplayNameGeneration.class);
        if (dynamicTest.isPresent() || displayNameGenerator.isPresent()) {
            return Optional.of(testIdentifier.getDisplayName());
        }

        return Optional.empty();
    }

    @Override
    public Optional<String> getDescription(TestIdentifier testIdentifier) {
        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        Optional<XrayTest> xrayTest = AnnotationSupport.findAnnotation(testMethod, XrayTest.class);

        return xrayTest
                .map(XrayTest::description)
                .filter(not(String::isEmpty));
    }

    protected Optional<Method> getTestMethod(final TestSource source) {
        if (source instanceof MethodSource) {
            return getTestMethod((MethodSource) source);
        }
        return Optional.empty();
    }

    protected Optional<Method> getTestMethod(final MethodSource source) {
        try {
            final Class<?> aClass = Class.forName(source.getClassName());
            return Stream.of(aClass.getDeclaredMethods()).filter(method -> MethodSource.from(method).equals(source))
                    .findAny();
        } catch (ClassNotFoundException e) {
            //logger.error(e, () -> "Could not get test method from method source " + source);
        }
        return Optional.empty();
    }
}

Then XmlReportWriter could initiate the provider and read id/key/sumary/description by calling getId/getKey/getSummary/getDescription methods. The user could override the default provider in the xray-junit-extensions.properties file (or in the other way, annotation?).

Then user can customize the provider like that:


import org.junit.jupiter.params.ParameterizedTest;
import org.junit.platform.commons.support.AnnotationSupport;
import org.junit.platform.engine.TestSource;
import org.junit.platform.launcher.TestIdentifier;

import java.lang.reflect.Method;
import java.util.Optional;

public class CustomXrayTestInfoReader extends DefaultXrayTestInfoReader {

    @Override
    public Optional<String> getKey(TestIdentifier testIdentifier) {
        Optional<String> defaultKey = super.getKey(testIdentifier);
        if (defaultKey.isPresent()) {
            return defaultKey;
        }

        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        Optional<ParameterizedTest> parameterizedTest = AnnotationSupport.findAnnotation(testMethod, ParameterizedTest.class);
        if (parameterizedTest.isPresent() && parameterizedTest.get().name().contains(":")) {
            return Optional.of(testIdentifier.getDisplayName().split(":")[0]);
        }
        return Optional.empty();
    }

    @Override
    public Optional<String> getSummary(TestIdentifier testIdentifier) {
        Optional<String> defaultSummary = super.getSummary(testIdentifier);
        if (defaultSummary.isPresent()) {
            return defaultSummary;
        }

        final Optional<TestSource> testSource = testIdentifier.getSource();
        final Optional<Method> testMethod = testSource.flatMap(this::getTestMethod);
        Optional<ParameterizedTest> parameterizedTest = AnnotationSupport.findAnnotation(testMethod, ParameterizedTest.class);
        if (parameterizedTest.isPresent() && parameterizedTest.get().name().contains(":")) {
            return Optional.of(testIdentifier.getDisplayName().split(":")[1].trim());
        }
        return Optional.empty();
    }
}

And use in the tests like that:

public class Play3Test {

    private static Stream<Arguments> dataSource() {
        return Stream.of(
                // first argument: Jira key
                // second argument: actual test parameter
                of("ABC-100", "PRODUCT_TABLE"),
                of("ABC-101", "ITEM_TABLE"),
                of("ABC-102", "USERS_TABLE"),
                of("ABC-103", "PRODUCT_TABLE")
        );
    }

    @XrayTest
    @ParameterizedTest(name = "{0}: Sorting {1} table")
    @MethodSource("dataSource")
    void sortTableTest(String _key, String tableName) {
        // _key param is ignored in the test code
        // tableName is used in the code

        // test code...
    }
}
bitcoder commented 3 months ago

@mk868 I like that last approach you suggested; it seems clean and flexible. "The user could override the default provider in the xray-junit-extensions.properties file " => seems feasible.

Do you want to make a PR for this?

mk868 commented 3 months ago

Sure, I'll prepare a PR soon

bitcoder commented 3 months ago

Sure, I'll prepare a PR soon

thanks, I'm with a bunch of stuff right now. Please include also tests for this new behavior. I will do a review in the end and if needed I can try to fill some gaps. thanks again

mk868 commented 3 months ago

So, I've prepared some initial implementation in PR, Could you check it and give your feedback?

bitcoder commented 3 months ago

So, I've prepared some initial implementation in PR, Could you check it and give your feedback?

Thanks, will do.

bitcoder commented 2 months ago

@mk868.

  1. can you please review the SonarCloud analysis that is mentioned on the PR?

Note: there are 3 initial issues that were "my fault" as they're related with names of variables like "test_id", "test_summary"...

  1. Can you update the README.md to reflect this new behaviour? I guess it needs a few changes, namely on the "Configuration", "How to use", and "Name of tests" (well, this later on perhaps not)
mk868 commented 2 months ago
  1. Yes, I see, fixed
  2. I've added a brief description and example of use
bitcoder commented 2 months ago

thanks. I'll have a more in-depth look next week

bitcoder commented 2 months ago

fixed on #55