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

Incorrect listener's onStart(ISuite suite) execution order in case of inner suites #1656

Open
7 tasks done
avarabyeu opened this issue Dec 28, 2017 · 25 comments
Open
7 tasks done

Comments

@avarabyeu
Copy link

avarabyeu commented Dec 28, 2017

TestNG Version

Note: only the latest version is supported

6.11

Expected behavior

The listener is called for each suite, if the parent suite contains child suites then the parent suites should run first before running the child suite.

Actual behavior

The child suites are first run before running the parent suite.

Is the issue reproducible on runner?

  • Shell
  • Maven
  • Gradle
  • Ant
  • Eclipse
  • IntelliJ
  • NetBeans

Test case sample

<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
<suite name="ParentSuite" parallel="classes" thread-count="10">

    <suite-files>
        <suite-file path="child_suite.xml"/>
    </suite-files>

    <test verbose="1" name="SomeTest">
        <classes>
            <class name="com.example.SomeTest"/>
        </classes>
    </test>

</suite>
@juherr
Copy link
Member

juherr commented Dec 28, 2017

Currently, suite-files is like an import, include <test> from specified suites and the result is only one suite.
That's why onStart is only called once.

@avarabyeu
Copy link
Author

@juherr actually it's called twice (for each suite file). Has the behaviour changed since 6.11?

@juherr
Copy link
Member

juherr commented Dec 29, 2017

Ok, two calls is an issue.

@krmahadevan What is the expected behavior of onStart in case of suite-files?

@krmahadevan
Copy link
Member

@juherr - I think that's the correct behavior.

As part of #1285 I think we changed this behavior

@juherr
Copy link
Member

juherr commented Dec 29, 2017

I am able to reproduce the order issue but, in fact, TestNG was always running child suites before parent suite (https://github.com/cbeust/testng/blame/master/src/main/java/org/testng/TestNG.java#L1191-L1199) and listeners were not correctly added before #1285.

@juherr
Copy link
Member

juherr commented Dec 29, 2017

6.13 should have fixed the duplicate issue with #1533

@juherr
Copy link
Member

juherr commented Dec 30, 2017

@avarabyeu
I can change the order behavior and run the master suite before the child suites but the master suite will finish before its children.
Will it be ok for you? A better child management implies big changes we will probably don't do.

Could you confirm that 6.13 is fixing the event duplication issue?

@avarabyeu
Copy link
Author

avarabyeu commented Jan 3, 2018

@juherr Can confirm that 6.13 does not have issue with duplication.

I can change the order behavior and run the master suite before the child suites but the master suite will finish before its children.

Yeah it would be easier for us to handle events in this case. Thank you!

@juherr
Copy link
Member

juherr commented Jan 5, 2018

@cbeust @krmahadevan WDYT if we change the order execution of suite/child?

@krmahadevan
Copy link
Member

@avarabyeu - Just trying to understand, how does the order of execution matter ? Why is it problematic if the child suites execute first and then the parent suite ?

@juherr - do we really need to change this ? We can easily have this sorted out at the test level itself by moving the contents of the master suite into a suite of its own, and then having the master suite just work as a suite of suites no ? Just trying to understand what we are going to get as benefits ?

@juherr
Copy link
Member

juherr commented Jan 8, 2018

@krmahadevan You will find more details on reportportal/reportportal#273
The goal is to find a short fix for the issue without breaking things.
With more time, I'd prefer to have proper child suites management but I'm not feeling it very important.
If I understand your solution, you propose to merge all child <test> into the parent <suite>, right?

@avarabyeu
Copy link
Author

avarabyeu commented Jan 8, 2018

@krmahadevan ReportPortal is a tool for real-time reporting of results from different test engines. So basically, its client handles events fired by testng listener and sends them to server. Order of that events matter because based on them ReportPortal fills internal data structures which are tree-based - for instance, it's impossible to create child before the parent.

@krmahadevan
Copy link
Member

@juherr

If I understand your solution, you propose to merge all child into the parent , right?

No. That is not what I meant. I am basically saying that the parent suite xml file should not have any <test> tag entries in it but only contain <suite-file> entries.

@krmahadevan
Copy link
Member

@avarabyeu - Thanks for sharing additional context. I am still not able to grasp the idea behind the order of execution being important, because TestNG does fire events before and after suite execution. So irrespective of whether child suites execute or the parent, the corresponding events get triggered no ?

@juherr
Copy link
Member

juherr commented Jan 8, 2018

No. That is not what I meant. I am basically saying that the parent suite xml file should not have any tag entries in it but only contain entries.

Ok! True, it is something we can check and warn against.

@avarabyeu
Copy link
Author

avarabyeu commented Jan 9, 2018

So irrespective of whether child suites execute or the parent, the corresponding events get triggered no ?

Yeah, but we build the same structure in our internal database - parent suite -> child suite. So if child suite starts earlier then parent, we simply cannot handle it on server side.

@krmahadevan
Copy link
Member

@avarabyeu - So what happens if you don't have any <test> tags at all in your master suite file but it has only <suite-file> tags in it ? Would that not work for you ?

I am basically trying to see what can be done to help you get past the problem without any code change.

@avarabyeu
Copy link
Author

@krmahadevan actually, this is good question. Issue came from our clients and i'm not really sure if they are able to adjust their testng.xml files in order to solve this integration issue. Let me check what happens if we have no in master suite.

@krmahadevan
Copy link
Member

Closing this issue for now. @avarabyeu Please comment back if you have an update on the query that I raised so that we can further triage this issue if necessary.

@ulmanA
Copy link

ulmanA commented Jul 31, 2018

@krmahadevan @avarabyeu
I tried to execute suite of suite-files, without any <test> tags in the master suite file and the problem still happened.
(tested on TestNG v6.14.3)

@krmahadevan
Copy link
Member

krmahadevan commented Jul 31, 2018 via email

@ulmanA
Copy link

ulmanA commented Jul 31, 2018

The same problem happened on version 7.0.0-SNAPSHOT
attaching example maven project
testng-reportportal-example.zip

@krmahadevan krmahadevan reopened this Aug 1, 2018
@krmahadevan
Copy link
Member

@ulmanA - The sample you shared doesn't say a lot of things (at-least it wasn't explicit for me).

You basically have a suite file called all-suites.xml which looks like below

<suite name="All Suites" verbose="2">
    <suite-files>
        <suite-file path="suite1.xml"/>
        <suite-file path="suite2.xml"/>
    </suite-files>
</suite>

And when I run this, I notice that suite1 gets executed first, then suite2, leading to all_suites completing itself.

...
... TestNG 7.0.0-SNAPSHOT by Cédric Beust (cedric@beust.com)
...
Suite 1 -> TEST 1
Suite 1 -> TEST 2

===============================================
All Suites (0)
Total tests run: 2, Passes: 2, Failures: 0, Skips: 0
===============================================
Suite 2 -> TEST 2
Suite 2 -> TEST 1

===============================================
All Suites (1)
Total tests run: 2, Passes: 2, Failures: 0, Skips: 0
===============================================

===============================================
All Suites
Total tests run: 4, Passes: 4, Failures: 0, Skips: 0
===============================================

So what is your expectation here ? Its not clear.

all_suites is visualized as a container of suites so its going to be finishing at the last.

@ulmanA
Copy link

ulmanA commented Aug 5, 2018

@avarabyeu
Please answer.

@nebehr
Copy link

nebehr commented Aug 13, 2018

@krmahadevan I am not the bug reporter but I would expect the following order of execution:

  1. ISuiteListener#onStart - all_suites
  2. ISuiteListener#onStart - suite1
  3. ISuiteListener#onFinish - suite1
  4. ISuiteListener#onStart - suite2
  5. ISuiteListener#onFinish - suite2
  6. ISuiteListener#onFinish - all_suites

So yes, all_suites should finish last but it should start first, because it contains all other suites! However, currently all_suites behaves not as a root container but as a container of any items not included in the nested suites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants