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

Parameters added in XmlTest during AlterSuiteListener not available in SuiteListener #2469

Closed
aditya-qapitol opened this issue Jan 28, 2021 · 5 comments
Labels
xml

Comments

@aditya-qapitol
Copy link
Contributor

@aditya-qapitol aditya-qapitol commented Jan 28, 2021

TestNG Version

7.3.0

Is the issue reproductible on runner?

  • Eclipse
  • IntelliJ

Test case sample

testng.xml

<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd" >

<suite name="HookableExample Suite">
    <test name="HookableListenerExample">
        <classes>
            <class name="com.example.test.SampleTest"></class>
        </classes>
    </test>
</suite>

AlterSuiteListener

public class TestNgAlterSuiteListener implements IAlterSuiteListener {
    @Override
    public void alter(List<XmlSuite> suites) {
        XmlSuite suite = suites.get(0);
        XmlTest xmlTest = suite.getTests().get(0);

        List<XmlTest> newXmlTests = new ArrayList<>();

        XmlTest newXmlTest = (XmlTest) xmlTest.clone();
        newXmlTest.setName("Name_1");
        newXmlTest.addParameter("param", "1");
        newXmlTest.setXmlClasses(xmlTest.getXmlClasses());
        newXmlTests.add(newXmlTest);

        newXmlTest = (XmlTest) xmlTest.clone();
        newXmlTest.setName("Name_2");
        newXmlTest.addParameter("param", "2");
        newXmlTest.setXmlClasses(xmlTest.getXmlClasses());
        newXmlTests.add(newXmlTest);

        suite.setTests(newXmlTests);
    }
}

SuiteListener

public class TestNgSuiteListener implements ISuiteListener {

    @Override
    public void onStart(ISuite suite) {
        for (XmlTest xmlTest : suite.getXmlSuite().getTests()) {
            System.out.println("onStart Name: " + xmlTest.getName());
            System.out.println("onStart Param: " + xmlTest.getLocalParameters().get("param"));
        }
    }
}

From the above SuiteListener code following is the behavior

Expected behavior

onStart Name: Name_1
onStart Param: 1
onStart Name: Name_2
onStart Param: 2

Actual behavior

onStart Name: Name_1
onStart Param: 2
onStart Name: Name_2
onStart Param: 2

@sgilson
Copy link
Contributor

@sgilson sgilson commented Jan 28, 2021

result.setParameters(getLocalParameters());

public Map<String, String> getLocalParameters() {
return m_parameters;
}

XmlTest.clone() creates a new test with a reference to the same local parameters map. Will fix by creating a copy of the local parameters and giving that to the clone instead.

@sgilson
Copy link
Contributor

@sgilson sgilson commented Jan 28, 2021

Looking at it more, this issue impacts several other fields. Since the method does not advertise support for deep cloning, I hesitate to call this a bug, just undesired behavior.

I'd be willing to look deeper into what else would need an update to support a deep clone, but perhaps someone more established could provide some insight?

@juherr juherr added the xml label Jan 28, 2021
@juherr
Copy link
Collaborator

@juherr juherr commented Jan 28, 2021

I agree it should work.
In fact, the XML model is aged and not enough strong.
But it is not easy to fix.

Could you propose a fix? Until it breaks things, we will be happy to merge it.

@sgilson
Copy link
Contributor

@sgilson sgilson commented Jan 28, 2021

I'll work on it. This certainly goes very deep given how many models are nested under the XmlTest.

The circular reference between XmlTest and XmlSuite may need some extra thought. My intuition is that cloning the test should not clone the parent suite. However, using the same suite would lead to a slightly inconsistent state where the cloned test has a parent suite but will not be executed since it is not listed in the suite's tests. As long as this is explicitly stated in the javadoc, does this behavior make sense?

aditya-qapitol added a commit to aditya-qapitol/testng that referenced this issue Jan 28, 2021
@aditya-qapitol aditya-qapitol mentioned this issue Jan 28, 2021
2 of 2 tasks complete
@aditya-qapitol
Copy link
Contributor Author

@aditya-qapitol aditya-qapitol commented Jan 28, 2021

@sgilson @juherr In my case I need a clone of the XmlTest with the same parent XmlSuite. Please check the PR for the fix.

aditya-qapitol added a commit to aditya-qapitol/testng that referenced this issue Jan 28, 2021
aditya-qapitol added a commit to aditya-qapitol/testng that referenced this issue Jan 28, 2021
aditya-qapitol added a commit to aditya-qapitol/testng that referenced this issue Feb 2, 2021
krmahadevan pushed a commit that referenced this issue Feb 3, 2021
Fix for "Parameters added in XmlTest during AlterSuiteListener not available in SuiteListener" 

Closes #2469
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants