Presto: Proposal to rework AbstractTestQueries/reduce reliance on product tests

[ebd2]

We've been having some internal discussions about reworking AbstractTestQueries with several goals:

1. Making tests run faster
2. Being able to break ATQ apart into separate classes to test e.g. basic operations like SELECT 1 + 1, aggregations, various joins, window functions, etc.
3. Folding some of the tempto tests that test basic engine functionality into an appropriate test class. For example, the window function tests that I wrote would be easier to both debug and run through IntelliJ if they were implemented the way that the ATQ tests are.
4. Limit connector-specific tests to testing functionality specific to each connector.

This discussion was motivated by @martint's https://github.com/prestodb/presto/issues/6910#issuecomment-274389525 on an earlier issue.

I talked this over with @maciejgrzybek and @alandpost and we think the work can be done as follows:

1. With the exception of TestHiveDistributedQueries and TestRaptorDistributedQueries, make all of the test classes that non-trivially extend ATQ extend AbstractTestQueryFramework or AbstractTestDistributedQueries instead. Test classes that trivially extend ATQ don't have any connector specific tests, and can be removed. The rationale for this is to remove tests that redundantly test core features of the Presto engine, but to retain those tests against 2 connectors to make it possible to identify test failures as being connector related (less likely) or core-Presto related (more likely).
2. Pull request for 1.
3. Create the infrastructure to break ATQ up into multiple classes, while retaining the ability to run tests from IntelliJ, as we have today.
   
   
   
   • @maciejgrzybek proposed using a static field to contain the query runner. Some quick and dirty work suggests that this is possible. Rationale: We don't want to recreate the query runner for every test class.
   
   
   • @alandpost proposed generating test classes to get around the matrix problem. E.g. create an AbstractJoinQueries class containing tests of join-related functionality and generate TestHiveDistributedJoinQueries and TestRaptorDistributedJoinQueries. Rationale: This retains the ability to run tests from within IntelliJ, and we have precedent for generating code for the parser. You already need to do a maven build to get IntelliJ in sync with the codebase.
   
   
4. Move many of the existing tempto sql convetion tests into new test classes using the infrastructure created in 3. Rationale: Large swaths of the existing sql testcases don't require the level of infrastructure that tempto provides. Further, this removes the requirement to keep test results in the repository, reducing the maintenance burden.
5. Pull Request for 3, 4.
6. Distribute existing tests in ATQ into test classes created in 4 or new test classes, as appropriate. Remove redundant test cases. Rationale: ATQ should be broken up into classes that test a specific feature in Presto.
7. Pull request for 7.
8. Improve connector-specific tests. At a minimum, these should include round-trip tests for all of the data types that a connector supports.
9. PR for 8.
10. Further improvements to connector-specific tests, possibly including validation of information schema queries, as suggested by @cawallin.

After the work the goal is that the tests are broken up as follows:

• Core presto functionality is tested by a number of test classes, each focused on some aspect of core functionality. These are run against 2 connectors.
• Connector-specific functionality is tested in small test classes that validate date type round tripping, correctness of results for queries that include push-down, etc.
• Tempto is used to test features that depend on infrastructure outside of Presto, e.g. Kerberos, LDAP, etc.

[ebd2]

@martint @electrum: I think this addresses most of the issues you raised in the discussion of #6910. I've hashed this out internally with Maciej, Alan, and Christina, and wrote it up for general review. If there's agreement that this is a fundamentally sound plan, I'd like to get started on the first couple pieces while we work out details on some of the stuff further down the list.

[maciejgrzybek]

AD 1. Just to make it clear, such a test would not need to use Raptor, nor Hive but for simplicity it's easier to exercise existing connector and issue a query in SQL instead of extensively mocking stuff. It's just an implementation detail that we use those two connectors.
AD 3. It does not have to be static field but some sort of singleton.

[electrum]

First off, thanks for looking at this. I fully appreciate this problem and have spent quite a lot of time thinking about this myself.

I don't think step 3 for breaking up AbstractTestDistributedQueries works. We already have 3 leaf classes: TestHiveDistributedQueries, TestRaptorDistributedQueries, TestRaptorDistributedQueriesBucketed. Suppose we split the base class into 10 classes. Now we go from 3 to 30 leaf classes. Each time we add a new base class, we need to add 3 more leaf classes. It also makes it nearly impossible to add another leaf class if we need to test another configuration (e.g., bucket vs non-bucket).

Internally, we have our sharded MySQL connector that extends AbstractTestQueries and (to my knowledge) tests things like index joins, streaming window functions, etc., that are not otherwise tested. We've definitely found bugs in the engine that are only caught by the sharded MySQL connector tests. So this is yet another required leaf class, bringing us to 4.

I also disagree with step 1. Moving more things to extend AbstractTestDistributedQueries is a step backwards. What we need is to extend AbstractTestIntegrationSmokeTest to cover everything for a connector so thatAbstractTestQueries` is not required. The hard part here is knowing what tests are needed. The best way would be to know what bugs people encounter while developing a connector that are caught by the existing tests, but unfortunately, that knowledge is lost.

[martint]

What we need is to extend AbstractTestIntegrationSmokeTest to cover everything for a connector so thatAbstractTestQueries` is not required.

Indeed. What I'd like to see (as suggested before and captured in the goals above) is feature-specific tests in separate classes (similar to how some of the planner tests work: TestReorderJoins, TestMergeWindows, etc). The goal of these tests would be to ensure the query engine (parser/analyzer/planner/execution) works as expected, and not to test any specific connector. To start with, we could move out all the tests that don't involve connectors.

AD 3. It does not have to be static field but some sort of singleton.

We should take a look at junit (version 5, in particular). I played a bit with junit 4 a while ago to create a test runner that makes it easier to do inject state into tests like JMH does it, with the option of scoping the state to the test, the class, or the whole suite: https://github.com/martint/presto/commit/ef7f759223fdc7f654f9fa51a819f680c512a2b1.

[findepi]

To start with, we could move out all the tests that don't involve connectors.

That sounds pretty obvious. If connector specific tests pass and feature specific tests pass too, everything should work and doesn't require further testing. Yet, there is #8165 -- feature specific test passes on many connectors, redis tests pass too, but feature specific test fails on redis. Just a thought, maybe running abundance of connector-agnostic tests allows exercising both engine and connectors more?

[ebd2]

I talked to @electrum offline about this last week, and did a little hacking to see how many tests in ATQ are completely independent of any connector functionality (i.e., select from values, select 1 + 1, etc). It's a hair under 200. I did this by altering the TPCH connector to throw UnsupportedOperationException in getTableHandle, etc.

Those could be moved out of ATQ today into, for the sake of argument, a whole new category of tests. The downside to doing that is that most of those tests are intermingled with tests that do hit the connector, possibly just for data and nothing else. I would suggest that moving them today would make it harder to follow their git history and context and create confusion as to where new tests should go.

Closer to @martint's original proposal would be to implement SimplestPossibleConnector, which does nothing more than produce data on demand. We could probably base it on the TPCH connector by selectively disabling anything beyond what's needed to actually get data to presto.

That gets us tests of Presto features, and ensures that we don't inadvertently hide presto bugs by using a connector that supports e.g. pushdown. We can also move the feature tests all at once, keeping related tests together. This also depends on either SimplestPossibleConnector being super cheap, or having the infrastructure to re-use it across test classes. Any test that can't be run against SimplestPossibleConnector is probably actually an integration test.

It's also useful to run this set of tests against AMoreFeaturefulConnector to ensure that Presto generates correct results when using, a connector that supports more than just producing data on demand. I originally proposed using Hive and Raptor, both of which fulfill this role, so that we could check them against each other. @electrum brought up the FB internal ShardedMySQLConnector as another connector that FB is likely to continue under any new scheme, given their reliance on it.

The eventual set of tests run under the remodeled ATQF should look something like this:

x | Data-only tests | Tests applicable to all more featureful connectors | SpecificConnectorATests | SpecificConnectorBTests
-|-------|-------|-------|-------
SimplestPossibleConnector | x | | |
AMoreFeaturefulConnector | x | x | |
SpecificConnectorA | | x | x |
SpecificConnectorB | | x | | x

And, as @martint suggested, we should really have a connector test suite that validates the behavior of connectors when poked directly via the SPI. This should at least clarify the contract between presto and connectors and help address @findepi's concerns about #8165. If the redis connector is violating the contract, we should catch that in the connector test. If it's Presto (and it's not clear to me from the discussion who's truly to blame), we should think about how me might test weird but legal connector behavior for specific cases.

Based on the hacking and offline discussions with @electrum, @maciejgrzybek, and @cawallin, maybe the revised plan should be:

1. Infra to break apart ATQ into multiple classes without recreating the QueryRunner every time. This seems to be highest risk/most contentious and getting a POC out for people to play with/debate seems like a reasonable plan.
2. Connector test suite. This can go in parallel with the debate on 1.
3. Create SimplestPossibleConnector. Move any tests in ATQ that can't run against it elsewhere based on what they require. Still doesn't require landing 1 to do.
4. Choose AMoreFeaturefulConnector (or several). Also doesn't depend on 1.
5. Improve connecvtor-specific tests as described in 8 at the very top. Doesn't depend on 1.
6. Distribute product tests as described in 4 at the top. Depends on 1.
7. Distribute remaining ATQ tests as described in 6 at the top. Depends on 1.

The goals remain largely unchanged, with the addition that we're explicitly testing presto when the presto code can't offload any work to the connector. Also, @findepi's comment about #8165 suggests that ensuring connectors comply with the SPI contract should be addressed before reducing the existing coverage of ATQ.

[ArturGajowy]

I'd say for 1. (Infra to break apart ATQ into multiple classes) we should use junit 5.

I created a junit 5 POC that seems to meet most functional criteria, including 'not re-creating the QueryRunner'.

It also doesn't have the drawback @electrum mentioned for test generation - the number of leaf classes doesn't change under that approach (is at most doubled for the migration period).

Here's my (probably biased) list of pros's and cons of the approaches. Feel free to edit and add yours.

JUnit-5 for the tests moved out of ATQ:

+ no code generation
+ no (or very little) custom tooling
+ good IDE support (assuming a current version of IntelliJ IDEA)
+ using the java built-in default interface methods facility
   lowers the bar for contributors
- need to migrate tests in question to JUnit
  * can be done test-by-test, need to migrate just the tests we want to pull up 
     in hierarchy
  * assertions are compatible between the libs due to them using AssertionError
     * migrating to JUnit assertions can be 
        [worth considering anyway](https://github.com/cbeust/testng/issues/543)
        - they seem more polished
  * most of the remaining test framework features have 1-1 equivalents
- need to create additional leaf classes for junit
  * at most doubling the leaf classes count, assuming we migrate all the leaf
     classes to the new approach

Test class generator:

- code generation
- need to maintain custom tooling
- need to recompile using mvn to generate the tests (unless we're able to plug 
   the annotation processor into IDE?)
- using custom tooling raises the bar for contributors
- no easy way to run all tests for a given feature due to proliferation of leaf classes
+ no need to migrate tests

[ebd2]

Having also started working on part 2 (connector unit tests) of this issue after putting up the original PR for the generation, I'm going to weigh in on the junit5 vs generation question.

First, for the upcoming ATQ breakdown, it's largely irrelevant which approach we take; individual tests in ATQ are by nature essentially compatible with every connector, and none need adaptation or filtering on a per-connector basis. In large part, this is because the ATQ tests are read-only.

The connector unit tests via the SPI are where it gets a lot hairier. The connector SPI exposes a large number of features that allow mutating the data or metadata exposed via the connector. I started writing tests that deal with ConnectorMetadata for Hive and PostgreSql and quickly ran into the need to deal with the fact that Hive supports pretty much every method that mutates the metadata, and PostgreSql supports none of them.

With that in mind here are the tradeoffs I see from a developer perspective:

Code Generation

- A small amount of additional infrastructure to maintain comprising:
   1. The annotation processor
   2. The runtime UnsupportedMethodInterceptor
   Today, minus the tests in presto-generation-tests, this represents about 300 lines of actual code.
- A maven build is required when
   1. Adding a new test class
   2. Adding a new connector
   3. Changing the supported or required features on a connector class or test class, respectively.
- Roll our own lifecycle management.
- More expensive to separate SPI and SQL tests; we'll need some additions to the infra to do this cleanly.
+ Connectors are annotated with what connector features are supported.
+ Test classes and methods can be annotated with what connector features are required.
+ Runtime unsupported test filtering means that connector-specific code doesn't need to know
   about what tests aren't supported. I.e. Connectors don't need to @Override unsupported methods.
+ The experience from IntelliJ remains largely similar to what it is today.
   1. Individual test classes, each representing a feature, can be run by selecting one concrete
      class, or all of them, from a dropdown on the feature.
   2. Similarly, you can navigate to a leaf class and run that test.
   3. If you want to run all of the leaf classes for a connector, you can run them by package.

One option that reduces both the amount of infra code and reduces the need to run maven builds would be to switch to purely runtime filtering rather than the hybrid filtering in the code today. I.e. generate leaf classes, even if the class requires features the connector doesn't support.

JUnit5

A couple general gripes that are going to resolve themselves in time:

• Until Java 9, test interfaces can't have private methods, which reduces the ability to do proper encapsulation. Java 9 will add the ability to add private methods to interfaces.
• IDE support is ... iffy. I lifted the maven code from Artur's example, but tried upgrading from M6 to RC2. Per the comments, this should be supported, but it resulted in not being able to run tests from the IDE because it doesn't find any. I did not dig to determine whether this is IntelliJ's fault or something screwed up in RC2, or whether there's another dependency that needs to be listed in the pom.xml file that isn't shown in the sample.
• There are wrinkles running the TestNG and PostgreSql from Maven. I'll look into it, but they aren't all running. It's not obvious where the problem lies at the moment.

A couple general observations that are likely to remain as they are today:

+ Lifecycle management is basically free.
+ Cheaper to separate SPI tests and SQL tests; the infrastructure to do so is free.
- Interfaces can't be instantiated (obviously), so anything a test method needs that would
   ordinarily be a field initialized in the constructor has to be obtained  a method call.

JUnit5 with tests grouped by feature (implemented in the PR)

Tests are grouped into classes by logical feature. E.g. ConnectorMetadata tests related to tables are all in the same class, even though not every connector supports everything that can be done with tables.

- Leaf classes for each connector need to override test methods for features that the connector 
   doesn't support (See MetadataTableTest/TestPostgreSqlMetadata)
   - As a consequence, leaf classes end up overriding a whole mess of unrelated unsupported tests.
- If you add a new test class, you need to touch every connector individually to get it to run the 
   new tests, and override the unsupported methods individually.

JUnit5 with tests grouped by required feature (not implemented)

Tests are grouped into classes by required feature E.g. any test requiring RENAME TABLE is in its own class.

- Test classes are likely to end up
   1. For tests requiring many features, extremely specific to a particular test, making it hard to run
      all of the tests related to a particular feature.
   2. For tests requiring few features, containing a grab-bag of all tests requiring those features 
      whether they're logically related or not (like ATQ is today).
+ Per-connector leaf classes don't need to be altered when new tests are added.

Not addressed yet

How to (eventually) limit running the ATQ tests to a couple of representative connectors

Generation

• Something involving TestNG tags? Generate all of the leaf classes, but limit which ones we run by tag.
• Add this to the annotations and processor, and only generate the tests we want to run.

I prefer the first; it reduces the infra code we have to maintain and means that if you want to run tests on a connector that we don't normally run on, the classes are already there.

Junit5

• This should be pretty cheap. Only implement the leaf classes for the connectors we want to run against.

[ebd2]

@martint, @electrum, @kokosing please take a look at this and the (not-ready-to-be-merged) PRs.

Opinions on the infrastructure options would be welcome.
I'd appreciate feedback from @martint in particular about whether this is the approach you had in mind for connector unit tests.

Please bear in mind that there are a lot of rough edges on both PRs. I didn't figure it was worth doing too much polishing until we pick a direction.

[kokosing]

I vote for junit5 approach:

• I did a quick pass over your two proposed approaches. I understood the junit5 easily, while the other it is much more complicated.
• the thing that sometimes developer has to call maven to generate something needed to run tests is the thing I would like to avoid. Usually, I work on several feature branches doing context switches frequently. Moreover often I checkout the code from PR I review. Even presto-parser could be annoying as it has to be run every time where there are some changes in AST. So I would love to avoid that.
• IMO the thing that junit5 is not yet fully supported by the IDE is intermittent as well as the fact the migration to jdk9 is unavoidable
• junit is kind of industry standard and comes with vast documentation which each developer can check instead of trying to understand when what is generated and when that it is called. Thanks to that we could leverage that knowledge on other aspect of testing, not only connectors and SQL while code generation is not portable to other Presto aspects.

I would suggest to start with junit5, and in case when we hit the wall we could invent something on our own. But that need to be a wall, not just a taste.

@losipiuk What do you think about this testing aproaches?

[ebd2]

@kokosing: You haven't addressed the question of the number of maintenance touch-points in the JUnit5 approach, by either proposing a better solution or arguing why it isn't a problem. Specifically the parts where changes to tests require touching basically every connector to either:

1. Make the new connector aware of the existence of the tests
2. Override unsupported tests

You also haven't explained what you find complicated about the generation approach. The generation and filtering is straightforward, and very few people are going to need to understand it, in much the same way that very few people need to understand the full internals of TestNG or JUnit.

I agree that the book-keeping is currently untidy in ATQF; I'd be happy to hear solutions for how to improve it. Again, this is something that few people should end up needing to touch, and specifically somebody implementing tests for a connector should not need to touch.

What I can say is complicated in the JUnit5 approach is the class hierarchy. I'm happy to hear suggestions about how to tidy it up too.

We're in agreement that having to run maven manually is a pain, I proposed a solution to reduce the reliance on running a command-line build.

Before we choose a path to go down, we need to understand the trade-offs inherent in both approaches. As I see it they boil down to:

Generation

Maintain custom tooling to get fewer manual touch points when changing tests

Junit5

Have a much more complex class hierarchy to get free book-keeping

[losipiuk]

@ebd2 thanks for great summary!

My initial though was that we want to avoid code generation if possible. That is based on my former work with annotation processors in Groovy. Probably irrelevant in many aspects but I recall that experience to be
much more painful that initially expected.
Also I am not big fan of TestNG library. It is not very actively developed right now, and code quality is poor. Provided that I would opt for migrating to JUnit.

When I read your list of pros and cons of two solution the two aspect seemed especially problematic if we want to follow the non-generation path.

1. Interfaces can't be instantiated (obviously), so anything a test method needs that would
   ordinarily be a field initialized in the constructor has to be obtained a method call.

2. Leaf classes for each connector need to override test methods for features that the connector
   doesn't support (See MetadataTableTest/TestPostgreSqlMetadata)

As for the 1. - maybe it is not a huge problem. ATQ does not define any fields right now. So need for that seems like trying to solve a problem we do not have yet.

As for the 2.

I looked into Junit5 documentation (which exists - which is pretty neat :) ) and it exposes the concept of ExecutionCondition.
This is a mechanism which allows you to perform runtime check to verify if given tests should be run in current execution context or not.
See: http://junit.org/junit5/docs/current/user-guide/#extensions-conditions.
The mechanism is used internally for implementing @Disabled annotation.
We could implement ExecutionCondition which validates if list of "required" features (specified as annotations on test) method is covered by the connector actually tested.
With this mechanism we would not need to override methods in leaf-test-classes to disable test on per-connector basis.

Given that I would vote for Junit5.

[kokosing]

• Interfaces can't be instantiated (obviously), so anything a test method needs that would
  ordinarily be a field initialized in the constructor has to be obtained a method call.

Why not provide a field with a method and then manage it within implementation of a test:

inteface BaseTest {
  @Test
  default void test() {
     assertNotNull(getObject());
  }
  Object getObject();
}

class SomeTest implements BaseTest {
   Object o;
   @Before public void setup() { o = ""; }
   @Before public void teardown() { o = null; }
   Object getObject() { return o; }
}

That way you have a custom code and you can manage the lifecycle of the object in the way you want.

---

Make the new connector aware of the existence of the tests

I do not think it is important point. New connector creation is a rare thing so it can be done manually, when developer creates a new connector she/he can choose the test suites manually.

---

Override unsupported tests

This is a bad or good thing. In case of generated tests, when you have a test suite which covers a feature A and developer creates a new connector and that new connector is satisfy feature A in 90%, so what she/he can do now? Either to do not run tests or split the feature A into A1, A2?

IMO if you are able create a list of features that given connector supports, then you also should be able to create a list of test suites for these features. Then instead of just saying that this connector supports that and then testing, you prove it by testing it right away. Having that said with proper test suites granularity there should be no need to override unsupported tests. Nevertheless, I think sometimes I could see a value in having such tests over weird feature granularity.

---

... very few people are going to need to understand it, in much the same way that very few people need to understand the full internals of TestNG or JUnit.

I am sorry, but I do not buy it. It is like currently saying that adding a test to ATQ require no knowledge about underlying classes like: AbstractTestQueryFramework or QueryRunner. It could be true, but once you do something more complicated then you need to take a look under the hood.

[losipiuk]

Why not provide a field with a method and then manage it within implementation of a test:

>

inteface BaseTest {
  @Test
  default void test() {
     assertNotNull(getObject());
  }
  Object getObject();
}

class SomeTest implements BaseTest {
   Object o;
   @Before public void setup() { o = ""; }
   @Before public void teardown() { o = null; }
   Object getObject() { return o; }
}

This would not work as you cannot use SomeTest as a building block of leaf test class (it is class and not an interface so you cannot use multiple inheritance). And you probably do not want to repeat the logic of setting up Object o for all the connectors.

But as I wrote maybe it is not a real problem we have to fight right now?

[ebd2]

Why not provide a field with a method and then manage it within implementation of a test:

That is exactly what I proposed. @losipiuk points out the tradeoffs in doing this, but I'm tentatively in agreement that we don't yet have a case where this is a problem.

This is a bad or good thing. In case of generated tests, when you have a test suite which covers a feature A and developer creates a new connector and that new connector is satisfy feature A in 90%, so what she/he can do now? Either to do not run tests or split the feature A into A1, A2?

Please take a look at https://github.com/prestodb/presto/pull/8671/files#diff-5f970463a19e2314ee3030d3acbcb489.

I would argue that these tests belong together, as they test related features. Note, however that support for mutating table metadata is fairly fine-grained. With Junit5, your choices are:

1. Break the tests apart into separate interface by feature. Now your tests are scattered out everywhere.
2. Override the unsupported methods. Now if you add support for e.g. renaming a table, you need to remember to remove the @ Override, or your feature isn't tested. Granted for generation, you need to remember to add the feature to the @ SupportedFeatures annotation, but that's one place (once per connector) instead of potentially many (once per method).
3. Use an ExecutionCondition as proposed by @losipiuk (Thanks for the pointer, I'd missed that in the docs). This solves the problems with both of the above, but requires custom code. One possible implementation relies on annotating test methods and connectors and looks substantially similar to the existing UnsupportedMethodInterceptor.

[electrum]

As for TestNG vs JUnit 5, from a framework perspective, it seems clear that JUnit 5 is the future and that we should move to it when it is ready and provides us benefit. Trying it out now would also be helpful to provide the JUnit project feedback before the final release.

So, we shouldn't frame this discussion in terms of JUnit 5 vs something else, but rather, after we move to JUnit 5, will it be sufficient, or will we need something else?

---

Assertions are a separate discussion from the testing framework. The parameter order assertEquals(actual, expected) seems clearly better, plus we have thousands of tests using it. Alternative matchers like AssertJ and Hamcrest also follow this order.

One option is to write our own wrapper class that delegates to JUnit or AssertJ. This would allow an easy migration by simply changing class imports. A downside is that it's non-standard, more code to maintain, and doesn't work with IDE inspections.

Another option is migrating everything to AssertJ. It's a huge huge win for complex assertions, but it seems needlessly verbose as a replacement for assertEquals, assertTrue, etc.

---

For conditional tests, we started doing that in AbstractDistributedTestQueries:

@Test
public void testView()
{
    skipTestUnless(supportsViews());
    ...
}

It uses the SkipException from TestNG. This exact approach is bad and isn't scalable -- something like ConnectorFeature is much better than lots of overridden methods. ExecutionCondition looks promising. I like that @Disabled (source) in JUnit 5 is built using this and is very simple code.

We should be able to specify @SupportedFeatures only once for the connector by using a mix-in interface (the ExecutionCondition would need to walk the interface hierarchy when looking for annotations).

[ebd2]

Following offline discussion with @electrum, I filed the following issue against JUnit 5: https://github.com/junit-team/junit5/issues/1009

[kokosing]

@ebd2 Thank you for your feedback. You did a great job!

It looks like that each options has its pros and cons. IMO either way go it will be better than it is now. Even if we make a mistake today, it will be easier to migrate once again later than it is today.

I can live with cons of both of them. Although, I still vote for junit5, I am ok with whatever you choose, either junit5 or code generation or even code generation with junit5 usage together.

[ebd2]

There's a new PR up that incorporates @losipiuk's suggestion to use ExecutionConditions to filter out the unsupported tests. That mitigates most of my maintainability concerns about JUnit5, and I'm going to stop maintaining the generation branch.

Please take a look at the PR and offer up opinions. Note that it won't work on Travis pending an airbase PR. I applied the patch in the linked PR to airbase version 64 (presto won't build against 67-SNAPSHOT) and made stuff work locally.

[smoyer64]

We've opened a more general discussion related to TestNG -> JUnit 5 migration at https://github.com/junit-pioneer/junit-pioneer/issues/62. Please come help us flesh out a "specification" that meets your needs!

[stale[bot]]

This issue has been automatically marked as stale because it has not had any activity in the last 2 years. If you feel that this issue is important, just comment and the stale tag will be removed; otherwise it will be closed in 7 days. This is an attempt to ensure that our open issues remain valuable and relevant so that we can keep track of what needs to be done and prioritize the right things.

[ArturGajowy]

I somehow feel this issue gets more important with each day that passes without closing it. I, a Wraith from The Past, pledge to haunt you every 2 years the stale bot questions this importance 👻