Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support dependUponMethods when resolving tests to launch #44

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
3 participants

fmisiak commented Dec 28, 2011

This is a set of changes to allow pulling automatically the test dependencies (dependUponMethods) when launching tests.
Let me know if theses changes are up to the quality you'd expect.

I'm really knew to testng-eclipse and the github process, so sorry in advance if I'm missing something obvious...

I'd like to go further in the future, to refactor/simplify the way we gather dependencies and maybe get a better performance by basing the computation on a JDT search (you'll see the base query in the above changes).

Owner

cbeust commented Dec 28, 2011

Hi,

Thanks for the patch, but... how different is it from the implementation I added to the plug-in to do this about a month ago?

fmisiak commented Dec 29, 2011

Your recent change only allowed to pull the "dependsOnGroups" dependencies.
This change is about pulling the dependencies specified in "dependsOnMethods".
It complements your recent change.
The way we want to use testng is such that the grouping feature was not practical enough. In essence we would have had to define one group per test method to achieve our goal.
Let me know if you need more info.

On Dec 28, 2011, at 6:00 PM, Cedric Beust reply@reply.github.com wrote:

Hi,

Thanks for the patch, but... how different is it from the implementation I added to the plug-in to do this about a month ago?


Reply to this email directly or view it on GitHub:
#44 (comment)

Owner

cbeust commented Dec 29, 2011

But... dependsOnMethods() has always been supported by the Eclipse plug-in, since it's the easy scenario (all the methods have to be on the current class or static on a different class). Maybe I'm misunderstanding, I'll take a closer look at your patch.

Thanks!

fmisiak commented Dec 29, 2011

Oh, now I realize I forgot to mention something important!
This fix allows to support fully qualified method names (as it is already supported by the testng runtime).
So when you launch a test method that depends on another test method defined in a different class they are both put in the generated test suite.

On Dec 28, 2011, at 7:14 PM, Cedric Beust reply@reply.github.com wrote:

But... dependsOnMethods() has always been supported by the Eclipse plug-in, since it's the easy scenario (all the methods have to be on the current class or static on a different class). Maybe I'm misunderstanding, I'll take a closer look at your patch.

Thanks!


Reply to this email directly or view it on GitHub:
#44 (comment)

Collaborator

missedone commented Dec 29, 2011

hi, cbeust and fmisiak

i'm not about to rush in, but here is also my fix to support full qualified method name:
https://github.com/missedone/testng-eclipse/commit/ef407baed523ff0cc7bcba881b975a40c7aaf515

this fix base on your fix of
e4e8c37 Support for dependsOnMethods()
dea343d GroupInfo -> DependencyInfo.

it also fix the NPE if there is typo/wrong method name in the "dependsOnMethods"
I don't think the user like to experience the Exception Dialog

this is the pull request i submitted several days ago:
#39

just for your reference, thanks

fmisiak commented Dec 29, 2011

Yes this is part of the fix to resolve this issue.
In the fix I'm proposing, I also solve the cases where this code was not invoked because of optimizations that were too aggressive (like only checking for dependsOnGroups() before invoking this computation, see my fix for details).
Also my fix includes a more robust way to determine where the dependsOnGroups/dependsOnMethods appears in the source code (it uses a JDT query instead of a text search based query) so that when the @test annotation spans multiple lines the test is still considered in the computation (see my fix for details).
Lastly, the generated XML test suite must includes a preserve-order=false otherwise the default order doesn't always work (you'll observe a nasty NPE when testng tries to evaluate the list of methods to launch when this happen).

On Dec 29, 2011, at 7:06 AM, Nick Tan reply@reply.github.com wrote:

hi, cbeust and fmisiak

i'm not about to rush in, but here is also my fix to support full qualified method name:
https://github.com/missedone/testng-eclipse/commit/ef407baed523ff0cc7bcba881b975a40c7aaf515

this is the pull request:
#39

just for your reference, thanks


Reply to this email directly or view it on GitHub:
#44 (comment)

fmisiak added some commits Jan 6, 2012

@fmisiak fmisiak Minor UI enhancements to result view:
 - refresh view for all updates (not just when an error occurs)
 - throttle UI updates to support above change (with a simple 500ms
delay)
 - show test method "on start" with "running" icon
dfb4bef
@fmisiak fmisiak Fix eclipse UI freeze.
Listening for the remote testng client to connect is no longer handled
in the eclipse UI thread ("main").It's now handled in a Job (which shows
up in the progress bar).
Note: blocking the UI thread to listen for a connection as adverse
consequences since this thread may hold some other locks. The problem
was really a deadlock which , for example, could easily be reproduced
when settings a breakpoints on FileNotFoundException.
b70b503

@cbeust cbeust closed this Aug 1, 2012

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