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

[Discussion] Implementation of statically typed data providers #2154

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

korobochka
Copy link
Contributor

@korobochka korobochka commented Sep 18, 2019

Hi!
I've always felt a bit "dirty" creating lots of Object[][] in data providers, and also sometimes wrong types get passed around leading to (somewhat) obscure runtime errors.

In this PR is the possible implementation of statically typed data providers, example of usage looks like this:

public class TypedDataProviderSample {

  @Test(dataProvider = "dp")
  public void doStuff(String a, int b) {
    // test body
  }

  @DataProvider(name = "dp")
  public void createData(@ParameterCollector TypedDataProviderSample test) {
    test.doStuff("test1", 1); // does not actually run the test
    test.doStuff("test2", 2); // only collects parameters
  }
}

Current implementation is by no means final, but I wanted to start a discussion about this, and if TestNG even needs such API.
However, this implementation works already.

Pros:

  • Types are checked by the compiler
  • Does not require massive modifications to TestNG, can be mixed with usual untyped Data Providers
  • Such style can be adapted to fibers/generators (project Loom)

Cons:

  • Requires runtime proxies, which brings additional dependency (ByteBuddy, or at least ASM). Can be circumvented by requiring test class to implement interface with single method for test, but it's not pretty.
  • Wrong method can be called in the data provider, which is checked only during runtime
  • Data providers become tied to the test method. But, honestly, I almost never reuse data providers between tests, so this is not a big issue.

What do you think? Would such API be useful for TestNG users? Maybe the same can be implemented better / in a different way?

Did you remember to?

  • Add test case(s) (no, need more tests)
  • Update CHANGES.txt (not needed for now)

@cbeust
Copy link
Collaborator

cbeust commented Sep 18, 2019

Interesting idea :-)

I think I would approach this a different way, maybe with an annotation processor that would generate code that only compiles if the types match.

Just thinking out loud, haven't really given this much thought.

@sskorol
Copy link
Contributor

sskorol commented Sep 19, 2019

@korobochka I've already concerned this before (when TestNG hasn't even migrated to Java 8). You can check test-data-supplier 3rd-party extension, which I've created to reduce DP pain. Note that it works with Java 11 modular apps as well. Moreover, there's an IntelliJ IDEA plugin for syntax highlight and basic navigation.

P.S. I don't believe it could be ever migrated to the current TestNG codebase due to additional dependencies and Java version differences.

@korobochka
Copy link
Contributor Author

@sskorol Your library looks good, but is solves a bit different problem (conveniently return Streams/Lists/other things) from the Data Provider. It still does not make sure (in compile time) that the types returned match test arguments.

@krmahadevan
Copy link
Member

@korobochka - Thinking loud.. What if we enhanced TestNG such that, this can reside outside of TestNG and when plugged in, TestNG would basically just invoke the customisation ? That way people would be able to wire in their way of dealing with statically typed data providers (today its byte code manipulation, tomorrow it could be something else). Do you think we could just take this to that direction ?

@sskorol
Copy link
Contributor

sskorol commented Sep 22, 2019

@korobochka I see your point now. What if we delegate this task to an IntelliJ plugin? At least developers will see highlighted areas on-fly, before compilation. Would it resolve your initial issue?

@juherr juherr requested review from krmahadevan and removed request for krmahadevan October 6, 2019 08:31
@juherr
Copy link
Member

juherr commented Oct 6, 2019

The idea is good but like @krmahadevan I think it should be provided in a 3rd party project.

But does TestNG provide an easy enough SPI for the injection of test parameters?

For the record, what is provided by JUnit5: https://blog.codefx.org/libraries/junit-5-parameterized-tests/

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.

5 participants