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

Merge [Container|Test]ExtensionContext into ExtensionContext #901

Closed
5 tasks done
sbrannen opened this issue Jun 28, 2017 · 4 comments
Closed
5 tasks done

Merge [Container|Test]ExtensionContext into ExtensionContext #901

sbrannen opened this issue Jun 28, 2017 · 4 comments

Comments

@sbrannen
Copy link
Member

sbrannen commented Jun 28, 2017

Overview

ConatinerExtensionContext is already just a marker interface, and with the changes being introduced in #419 the TestExtensionContext will soon contain a single Optional<Throwable> getTestException() method.

With these points in mind, now may be an opportune time to take the plunge and completely remove both the ConatinerExtensionContext and TestExtensionContext APIs and allow extension authors to rely solely on the ExtensionContext API.

Related Issues

Deliverables

  • Move getTestException() from TestExtensionContext to ExtensionContext.
  • Remove the ContainerExtensionContext API.
  • Remove the TestExtensionContext API.
  • Merge ContainerExecutionCondition and TestExecutionCondition into a single, general purpose ExecutionCondition whose method accepts a sole ExtensionContext argument.
@sbrannen
Copy link
Member Author

@junit-team/junit-lambda This issue is open for team discussion within the 5.0 M5 time frame.

@sbrannen
Copy link
Member Author

sbrannen commented Jun 28, 2017

Thinking out loud...

One thing I find at least slightly controversial about the proposal is the merging of ContainerExecutionCondition and TestExecutionCondition.

I know that the same contextual information will available to extension authors; however, extension authors may wish to implement entirely different logic for the container level and the test level.

Perhaps it's a non-issue, but it's definitely something to keep in mind.

This of course ties into any decision made in conjunction with #625.

@sbrannen
Copy link
Member Author

Added a sub-deliverable regarding getExecutionException() and #899.

@marcphilipp
Copy link
Member

in progress

marcphilipp added a commit that referenced this issue Jun 30, 2017
marcphilipp added a commit that referenced this issue Jun 30, 2017
@ghost ghost removed the status: in progress label Jul 1, 2017
nipafx pushed a commit to junit-pioneer/junit-pioneer that referenced this issue Aug 17, 2017
The merge of `TestExecutionCondition` and
`ContainerExecutionCondition` to `ExecutionCondition` caused compile
errors in `OsCondition` and related classes and tests. This is now
fixed.

Upstream issue: junit-team/junit5#901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants