testng-team / testng

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

expectedExceptions mismatch because of wrapping (regression 7.0.0 to 7.1.0) #2235

Closed Pr0methean closed 4 years ago

Pr0methean commented 4 years ago

TestNG Version

7.1.0

Expected behavior

When the method under test throws an IllegalArgumentException, a test with expectedExceptions = IllegalArgumentException.class passes.

Actual behavior

The test fails with:

FAILED: testConvertWrongNumberOfBytesToInts
org.testng.TestException: 
Expected exception of type class java.lang.IllegalArgumentException but got org.testng.internal.InvokeMethodRunnable$TestNGRuntimeException: java.lang.IllegalArgumentException: Number of input bytes must be a multiple of 4.
    at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
    at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:816)
    at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:146)
    at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: org.testng.internal.InvokeMethodRunnable$TestNGRuntimeException: java.lang.IllegalArgumentException: Number of input bytes must be a multiple of 4.
    at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:53)
    at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:71)
    at org.testng.internal.InvokeMethodRunnable.run(InvokeMethodRunnable.java:35)
    at org.testng.internal.MethodInvocationHelper.invokeWithTimeoutWithNoExecutor(MethodInvocationHelper.java:314)
    at org.testng.internal.MethodInvocationHelper.invokeWithTimeout(MethodInvocationHelper.java:280)
    at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:603)
    ... 9 more
Caused by: java.lang.IllegalArgumentException: Number of input bytes must be a multiple of 4.
    at io.github.pr0methean.betterrandom.util.BinaryUtils.convertBytesToInts(BinaryUtils.java:104)
    at io.github.pr0methean.betterrandom.util.BinaryUtilsTest.testConvertWrongNumberOfBytesToInts(BinaryUtilsTest.java:96)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:134)
    at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:47)

Is the issue reproductible on runner?

Test case sample

Test: https://github.com/Pr0methean/BetterRandom/blob/6df10c5b6e8ec24dea4275bf2b487dd9aecdc369/betterrandom/src/test/java/io/github/pr0methean/betterrandom/util/BinaryUtilsTest.java#L93 Code under test throws the exception here: https://github.com/Pr0methean/BetterRandom/blob/ca5deaff0497db33d7769bdf697af9852389f151/betterrandom/src/main/java/io/github/pr0methean/betterrandom/util/BinaryUtils.java#L104

(This version of the test passes on TestNG 7.0.0.)

krmahadevan commented 4 years ago

@Pr0methean - I looked at the test that you shared and created a similar test which looks like below

import java.io.File;
import org.testng.Assert;
import org.testng.TestNG;
import org.testng.annotations.Test;

public class ExampleTestCase {

  @Test(timeOut = 1000, expectedExceptions = IllegalArgumentException.class)
  public void testMethod() {
    String version = new File(TestNG.class.getProtectionDomain().getCodeSource().getLocation().getFile()).getParentFile().getName();
    Assert.assertEquals(version, "7.1.0");
    throw new IllegalArgumentException("foo");
  }
}

I am not able to simulate the problem with this sample.

I am not able to run the test from your codebase, because despite having only JDK8, for some reason IntelliJ keeps believing that I have JDK9 and keeps giving me some error.

Pr0methean commented 4 years ago

The test passes when I run it in IntelliJ IDEA; this bug may be specific to the Maven runner, or IDEA may be ignoring the pom.xml and using an old version of TestNG. (NB: I'm using JDK11 in IDEA.)

krmahadevan commented 4 years ago

Ok.. I think I got the problem.

Here's a sample that can be used to reproduce this issue

import org.testng.annotations.Test;

public class ExampleTestCase {

  @Test(timeOut = 1000, expectedExceptions = IllegalArgumentException.class)
  public void testMethod() {
    throw new IllegalArgumentException("foo");
  }
}
import java.util.Collections;
import org.testng.Assert;
import org.testng.TestListenerAdapter;
import org.testng.TestNG;
import org.testng.annotations.Test;
import org.testng.xml.XmlClass;
import org.testng.xml.XmlSuite;
import org.testng.xml.XmlSuite.ParallelMode;
import org.testng.xml.XmlTest;

public class IssueTest {

  @Test
  public void testMethod() {
    TestNG testng = new TestNG();
    XmlSuite xmlSuite = new XmlSuite();
    xmlSuite.setName("main");
    xmlSuite.setParallel(ParallelMode.METHODS);
    XmlTest xmlTest = new XmlTest(xmlSuite);
    xmlTest.setName("prngs");
    xmlTest.setXmlClasses(Collections.singletonList(new XmlClass(ExampleTestCase.class.getName())));
    testng.setXmlSuites(Collections.singletonList(xmlSuite));
    TestListenerAdapter listener = new TestListenerAdapter();
    testng.addListener(listener);
    testng.run();
    Assert.assertTrue(listener.getFailedTests().isEmpty());
  }
}
krmahadevan commented 4 years ago

@Pr0methean - The issue is reproducible when you run a testng suite that looks like below

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="Main" parallel="methods">
  <test name="PRNGs">
    <classes>
      <class name="io.github.pr0methean.betterrandom.util.BinaryUtilsTest">
        <methods>
          <include name="testConvertWrongNumberOfBytesToInts"/>
        </methods>
      </class>
    </classes>
  </test>
</suite>

The code path seems to be linked to enabling parallel execution, of a test method that has a time out defined and also has an expected exception.

Pr0methean commented 4 years ago

That would explain it; all the tests in BinaryUtilsTest are of static methods that are pure functions, so of course I let them run in parallel!