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.

Sign up for GitHub

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

Jump to bottom

Duplicate Tests being added to XmlTests if XmlTest is used in the parameters of annotated methods #1994

Open
ApexK opened this issue Dec 31, 2018 · 2 comments · May be fixed by #2001
Open

Duplicate Tests being added to XmlTests if XmlTest is used in the parameters of annotated methods #1994

ApexK opened this issue Dec 31, 2018 · 2 comments · May be fixed by #2001
Assignees
@krmahadevan
Labels
Feature: injection

Comments

@ApexK
Copy link

ApexK commented Dec 31, 2018 •
edited by krmahadevan

TestNG Version

v7

Expected behavior

Current XmlTest is returned

Actual behavior

Duplicate test is added to XmlTest

Is the issue reproductible on runner?

  • [] Shell
  • [] Maven
  • [] Eclipse

Test case sample

For example, use the following @BeforeTest

@BeforeTest(alwaysRun = true)
@Parameters({ "browser"})
 public void beforeTest(@Optional XmlTest xmlTest, @Optional String browser) {
      System.setProperty("webdriver.chrome.driver","/path/to/chromedriver");
      DesiredCapabilities c=DesiredCapabilities.chrome();
      ChromeOptions options = new ChromeOptions();
      options.addArguments("options");
}

Run any sample suite with around 5 test cases
Now after the tests are executed, check the total number of tests using debugging mode in m_tests. It shows 8 tests which include duplicate tests being added which contain empty xmlClasses
image

These duplicate/ empty test cases are also causing random NullPointer Exceptions at the end of the tests. These NPEs are however not reproducible all the time.

Attached is a small zip file with a sample maven project. You can reproduce the duplicate tests issue by just running the testng.xml file from eclipse and checking the m_tests at the AfterSuite methods by using ITestContext like the below:

@AfterSuite
public void afterSuite(ITestContext context) {
	  System.out.println("afterSuite");
}

Please let me know if any other info is needed

DuplicateTests.zip
@krmahadevan krmahadevan self-assigned this Jan 1, 2019
krmahadevan added a commit to krmahadevan/testng that referenced this issue Jan 1, 2019
@krmahadevan
Streamline native injection w.r.t cloning
aa992b0 
Closes testng-team#1994

Currently as part of supporting native injection
TestNG resorts to invoking the “clone()” method 
on the parameter being natively injected.

This becomes a problem because XmlTest is the only
internal TestNG object that implements the Cloneable
interface wherein it resorts to something like a deep
copy and adds the cloned instance to the XmlSuite.

This causes a lot of phony XmlTest objects to be 
added up to the suite and causes issues as detailed
in the bug.

Fixed this by introducing a marker interface to 
indicate to TestNG to skip cloning and use the object
as is. Annotated “XmlTest” with this annotation 
so that we dont clone xmltest but instead use it 
as is.

Yes this will expose the XmlTest object, but its 
already exposed to the end user via various different
means. So I guess we are ok here.
@krmahadevan krmahadevan mentioned this issue Jan 1, 2019
Closed
2 tasks
krmahadevan added a commit to krmahadevan/testng that referenced this issue Jan 6, 2019
@krmahadevan
Streamline native injection w.r.t cloning
96ea07b 
Closes testng-team#1994

Currently as part of supporting native injection
TestNG resorts to invoking the “clone()” method 
on the parameter being natively injected.

This becomes a problem because XmlTest is the only
internal TestNG object that implements the Cloneable
interface wherein it resorts to something like a deep
copy and adds the cloned instance to the XmlSuite.

This causes a lot of phony XmlTest objects to be 
added up to the suite and causes issues as detailed
in the bug.

Fixed this by introducing a marker interface to 
indicate to TestNG to skip cloning and use the object
as is. Annotated “XmlTest” with this annotation 
so that we dont clone xmltest but instead use it 
as is.

Yes this will expose the XmlTest object, but its 
already exposed to the end user via various different
means. So I guess we are ok here.
@krmahadevan krmahadevan linked a pull request Jan 6, 2019 that will close this issue
Open
2 tasks
krmahadevan added a commit to krmahadevan/testng that referenced this issue Jan 8, 2019
@krmahadevan
Streamline native injection w.r.t cloning
fa4d026 
Closes testng-team#1994

Currently as part of supporting native injection
TestNG resorts to invoking the “clone()” method 
on the parameter being natively injected.

This becomes a problem because XmlTest is the only
internal TestNG object that implements the Cloneable
interface wherein it resorts to something like a deep
copy and adds the cloned instance to the XmlSuite.

This causes a lot of phony XmlTest objects to be 
added up to the suite and causes issues as detailed
in the bug.

Fixed this by introducing a marker interface to 
indicate to TestNG to skip cloning and use the object
as is. Annotated “XmlTest” with this annotation 
so that we dont clone xmltest but instead use it 
as is.

Yes this will expose the XmlTest object, but its 
already exposed to the end user via various different
means. So I guess we are ok here.
@ApexK
Copy link
Author

ApexK commented Jan 24, 2019 •
edited

@krmahadevan I don't mean to rush but would be nice to know if there has been any update on this?

@krmahadevan
Copy link
Member

@ApexK - I have raised a PR to fix this issue. But me and @juherr haven't been able to arrive at a consensus on what we think would be the right way of fixing this.

@cbeust - Can you please chime in and provide your insights. I would really like to close this PR
Need your comments here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@krmahadevan krmahadevan

Labels
Feature: injection
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Streamline native injection w.r.t cloning krmahadevan/testng
2 participants
@krmahadevan @ApexK