testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.98k stars 1.02k forks source link

DataProvider modifies parameters of type Method #2189

Closed galactoise closed 4 years ago

galactoise commented 4 years ago

TestNG Version

6.14.3

Expected behavior

Objects injected from a DataProvider are passed verbatim without modification.

Actual behavior

Objects of type Method.class, which are added to the DataProvider's Object[][], end up being dropped and replaced with a different instance of Method describing the test method about to be invoked. Additionally, the following warning is printed:

:::WARNING:::
Missing one or more parameters that are being injected by the data provider. Please add the below arguments to the method.
Method: testMethodWithDataProvider([Parameter{index=0, type=java.lang.reflect.Method, declaredAnnotations=[]}])
Arguments: [(java.lang.reflect.Method) public void mypackage.DataProviderWithMethodTest.myMethod()]

Is the issue reproductible on runner?

Test case sample

package mypackage;

import java.lang.reflect.Method;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import static org.testng.Assert.assertEquals;

public class DataProviderWithMethodTest {
    public void myMethod() {}

    @Test //Passes when no DataProvider used
    public void testMethodWithoutDataProvider() throws NoSuchMethodException, SecurityException {
        Method m = DataProviderWithMethodTest.class.getMethod("myMethod");
        assertEquals(m.getName(),"myMethod");
    }

    @DataProvider //References Method object in exact same way as test above
    public Object[][] methodProvider() throws NoSuchMethodException, SecurityException {
        return new Object[][] {
            {DataProviderWithMethodTest.class.getMethod("myMethod")}
        };
    }

    @Test(dataProvider="methodProvider") //Fails unexpectedly
    public void testMethodWithDataProvider(Method m) {
        assertEquals(m.getName(),"myMethod");
    }
}

Note: The fact that all of the terminology here is overloaded makes it exceedingly impossible to search for other people who have had this issue in the past.

krmahadevan commented 4 years ago

@galactoise - Please add @NoInjection annotation to your method parameter of your data driven test, to ensure that TestNG disables the native injection and instead relies on the data provider to feed in the test method. Also please make sure that you are using 7.0.0 (latest released version of TestNG)

Here's a sample that demonstrates

import java.lang.reflect.Method;
import org.testng.annotations.DataProvider;
import org.testng.annotations.NoInjection;
import org.testng.annotations.Test;

import static org.testng.Assert.assertEquals;

public class DataProviderWithMethodTest {
  public void myMethod() {}

  @Test //Passes when no DataProvider used
  public void testMethodWithoutDataProvider() throws NoSuchMethodException, SecurityException {
    Method m = DataProviderWithMethodTest.class.getMethod("myMethod");
    assertEquals(m.getName(),"myMethod");
  }

  @DataProvider //References Method object in exact same way as test above
  public Object[][] methodProvider() throws NoSuchMethodException, SecurityException {
    return new Object[][] {
        {DataProviderWithMethodTest.class.getMethod("myMethod")}
    };
  }

  @Test(dataProvider="methodProvider") //Fails unexpectedly
  public void testMethodWithDataProvider(@NoInjection Method m) {
    assertEquals(m.getName(),"myMethod");
  }
}
galactoise commented 4 years ago

Hi @krmahadevan, that's a neat feature, and as I noted, it was exceedingly hard to google whether this was intended behavior or not because of the way the terminology is overloaded. So, I'm cool with this being closed as "by design". A couple notes, though:

  1. Your documentation needs to be updated. Now that I know this is a feature that exists, I decided to read up about it, but the table here: https://testng.org/doc/documentation-main.html#native-dependency-injection says that injection is not something that happens on Method objects in @Test annotated methods, which is untrue per the example above.
  2. It's not clear to me what value is provided by injecting a reference to the test method into the test method itself. What does this grant the user that they didn't already have by inspecting the call stack?
  3. I realize that for backwards compatibility reasons this probably isn't something you can ever change, but it feels like the design here - asking the user to actively opt out of injection - is a little bit backwards. Seems like that should've been an opt-in, and explicitly declared data providers should always be given precedence. In my case, the value I was trying to pass through was dropped on the floor with no notification from TestNG that it was going to discard my input. If you can't switch this to be opt-in in the future, it seems like at the very least this should cause the test case to skip and an exception should be thrown explaining that there were two different input sources fighting over who was going to supply the Test method's parameter.