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

TestResult does not support the expansion of existing statuses. #1945

Open
shaburov opened this issue Nov 1, 2018 · 11 comments
Open

TestResult does not support the expansion of existing statuses. #1945

shaburov opened this issue Nov 1, 2018 · 11 comments
Milestone

Comments

@shaburov
Copy link
Contributor

shaburov commented Nov 1, 2018

TestNG Version

testng-7.0.0-beta1

1 Expected behavior (? class status mapper for TestNG)

For example enum class

public class Example1 {

    public interface ITestResultStatusMapper<T> {

        T getStatus(int intStatus);

        int getStatusID(T status);

        String getStatusName(T status);

    }

    public static void main(String[] args) {
        ITestResultStatusMapper<StatusEnum> enumMapper = new ITestResultStatusMapper<StatusEnum>() {
            @Override
            public StatusEnum getStatus(int intStatus) {
                StatusEnum status = StatusEnum.getStatus(intStatus);
                return status.equals(UNKNOWN_STATUS) ? status.withId(intStatus) : status;
            }
            @Override
            public int getStatusID(StatusEnum status) {
                return status.getId();
            }
            @Override
            public String getStatusName(StatusEnum status) {
                return status.name();
            }
        };
        TestNG testNG = new TestNG();
        testNG.setResultStatusMapper(enumMapper);
        testNG.run();
    }

    public enum StatusEnum {
        SUCCESS (1),
        FAILURE (2),
        /* ... */
        EXPECTED_FIX (12),
        UNKNOWN_STATUS (-1);

        private int id;

        StatusEnum(int id) {
            this.id = id;
        }

        public StatusEnum withId(int id) {
            this.id = id;
            return this;
        }

        public int getId() {
            return id;
        }

        public static StatusEnum getStatus(int id) {
            for (StatusEnum value : StatusEnum.values()) {
                if (value.id == id) {
                    return value;
                }
            }
            return UNKNOWN_STATUS;
        }
    }
}

2 Expected behavior (Integer status mapper for TestNG)

public class Example2 {

    public static void main(String[] args) {
        ITestResultStatusMapper mapper = new ITestResultStatusMapper() {
            public String getStatus(int status) {
                if (status == 12) {
                    return "EXPECTED_FIX";
                }
                return ITestResultStatusMapper.super.getStatus(status);
            }
        };
        TestNG testNG = new TestNG();
        testNG.setResultStatusMapper(mapper);
    }

    public interface ITestResultStatusMapper {
        default String getStatus(int status) {
            switch (status) {
                case SUCCESS:
                    return "SUCCESS";
                case FAILURE:
                    return "FAILURE";
                case SKIP:
                    return "SKIP";
                case SUCCESS_PERCENTAGE_FAILURE:
                    return "SUCCESS WITHIN PERCENTAGE";
                case STARTED:
                    return "STARTED";
                case CREATED:
                    return "CREATED";
                default:
                    return "UNKNOWN_STATUS_" + status;
            }
        }
    }
}

3 Expected behavior (Less strict toString method)

return "UNKNOWN_STATUS_" + status;

  private static String toString(int status) {
    switch (status) {
      case SUCCESS:
        return "SUCCESS";
      case FAILURE:
        return "FAILURE";
      case SKIP:
        return "SKIP";
      case SUCCESS_PERCENTAGE_FAILURE:
        return "SUCCESS WITHIN PERCENTAGE";
      case STARTED:
        return "STARTED";
      case CREATED:
        return "CREATED";
      default:
        return "UNKNOWN_STATUS_" + status;
    }
  }

Actual behavior (exception)

throw new TestNGException("Encountered an un-defined test status of [" + status + "].");

  private static String toString(int status) {
    switch (status) {
      case SUCCESS:
        return "SUCCESS";
      case FAILURE:
        return "FAILURE";
      case SKIP:
        return "SKIP";
      case SUCCESS_PERCENTAGE_FAILURE:
        return "SUCCESS WITHIN PERCENTAGE";
      case STARTED:
        return "STARTED";
      case CREATED:
        return "CREATED";
      default:
        throw new TestNGException("Encountered an un-defined test status of [" + status + "].");
    }
  }
@krmahadevan
Copy link
Member

@shaburov - The code snippets in this issue, are they supposed to be proposing the new changes? I didn't quite understand.

Also how would one cater to the needs of the listeners that are supposed to be invoked when a test passes or fails or skips or succeeds within percentage etc.,

I believe there were previously also cases wherein this was asked for and I dont remember us reaching a conclusion on that. @juherr @cbeust thoughts ?

@juherr
Copy link
Member

juherr commented Nov 1, 2018

Why not for the idea but what is the fonctionnal need behind it?

@shaburov
Copy link
Contributor Author

shaburov commented Nov 6, 2018

It is much more convenient to use a ready-made solution TestResult, than to invent a bicycle.
I had to.
image

@juherr
Copy link
Member

juherr commented Nov 6, 2018

I like your screenshot! 👍

But 2 things:

  • I'd like to replace old fashion int status by enum ones. Any idea for having both enum and the expansion you'd like?
  • How do you expect that TestNG will use status it doesn't know?

@juherr juherr added this to the 8.0.0 milestone Jan 18, 2022
@juherr
Copy link
Member

juherr commented Jan 18, 2022

3 years later, I still want an enum :D

The current feature request could be done if we consider a SPI on the TERMINATED status (where SUCCESS, FAILURE, SKIP and SUCCESS_PERCENTAGE_FAILURE are just default configurations of it).

Currently planned for 8.x because it may change the current API.

Ping @krmahadevan

@shaburov
Copy link
Contributor Author

@juherr
Enum is hard to extend :)
I suggest implementing the default statuses via an interface. And in the API implementation, use interfaces instead of enums. In this case, users will be able to use their customized list of statuses through the API.

public interface IStatus {

    String name();

    public static enum DefaultStatuses implements IStatus {

        SUCCESS,
        FAILURE,
        SKIP,
        SUCCESS_PERCENTAGE_FAILURE

    }
    
}

@juherr
Copy link
Member

juherr commented Jan 20, 2022

Yes, I think we are saying the same thing.

The current TestNG status mix the test states (CREATED -> STARTED -> [RUNNING] -> [FINISHED]) and the reason of the finished state (SUCCESS, FAILURE, SKIP, SUCCESS_PERCENTAGE_FAILURE).

If I understand what you are asking for then you just want a way to specify the reason of the finished state.

Caution: status may be hardcoded in TestNG integrations (IDE, build tools, ...).

@shaburov
Copy link
Contributor Author

shaburov commented Jan 24, 2022

I'm only talking about test execution statuses and the possibility of their extension. Initially, in the task, the problem is in the exception.

Actual behavior (exception) TestResult

  private static String toString(int status) {
    switch (status) {
      case SUCCESS:
        return "SUCCESS";
      case FAILURE:
        return "FAILURE";
      case SKIP:
        return "SKIP";
      case SUCCESS_PERCENTAGE_FAILURE:
        return "SUCCESS WITHIN PERCENTAGE";
      case STARTED:
        return "STARTED";
      case CREATED:
        return "CREATED";
      default:
        throw new TestNGException("Encountered an un-defined test status of [" + status + "].");
    }
  }

An exception in the default block does not allow the use of custom statuses.
I suggest replacing the exception with return "UNKNOWN_STATUS_" + status;
And in a good way, remove the static modifier, since the method is used in one place inside the non-static toString() method.

Ideally, it would be nice to be able to extend statuses with a status mapper, but this is not required.

And in a completely ideal world, if we make a product through the API, then the user should be able to register his own implementation of the ITestResult interface. And at the same time be able to extend or change the implementation of TestResult, which involves using the protected modifier instead of private. But this is my subjective opinion.

@juherr
Copy link
Member

juherr commented Jan 24, 2022

Yes, by luck it is not possible and we won't have to support it 🙏

Just open it in the current design is not the good solution at the good problem you highlight.

@shaburov
Copy link
Contributor Author

In any case, in my opinion, the hardcode of statuses without the possibility of expanding the list is not the best solution.

@juherr
Copy link
Member

juherr commented Jan 29, 2022

The engine manages a state machine and I don't see any use case that justifies to implement this complexity.
What I understand you ask for is a way to parameter the final state.

The current design issue is that it mixes the 2 concepts into a single name.

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

No branches or pull requests

3 participants