Bug 429459 - Extend no-run option to provide more coverage of possible error situations
Summary: Extend no-run option to provide more coverage of possible error situations
Status: CLOSED FIXED
Alias: None
Product: Jubula (Archived)
Classification: Technology
Component: RC (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Maissmaallsmyss Maulhs-Vvuillss CLA
QA Contact: Oliver Goetz CLA
URL:
Whiteboard:
Keywords:
Depends on: 436061
Blocks:
  Show dependency tree
 
Reported: 2014-03-03 08:42 EST by Maissmaallsmyss Maulhs-Vvuillss CLA
Modified: 2014-06-05 08:52 EDT (History)
4 users (show)

See Also:


Attachments
The patch (57.20 KB, patch)
2014-03-03 08:42 EST, Maissmaallsmyss Maulhs-Vvuillss CLA
no flags Details | Diff
The patch for norun extention (22.85 KB, patch)
2014-04-25 06:51 EDT, Maissmaallsmyss Maulhs-Vvuillss CLA
no flags Details | Diff
The patch for norun extention (56.97 KB, patch)
2014-04-25 07:06 EDT, Maissmaallsmyss Maulhs-Vvuillss CLA
no flags Details | Diff
Changed patch (56.62 KB, application/octet-stream)
2014-04-29 12:05 EDT, Maissmaallsmyss Maulhs-Vvuillss CLA
no flags Details
patch with merged changes after other testexec patch apply (57.39 KB, patch)
2014-04-30 08:08 EDT, Maissmaallsmyss Maulhs-Vvuillss CLA
sebastian.jubula: iplog+
Details | Diff
Patch for handling NPE caused by changes in no-run option (8.91 KB, patch)
2014-05-05 08:01 EDT, Maissmaallsmyss Maulhs-Vvuillss CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maissmaallsmyss Maulhs-Vvuillss CLA 2014-03-03 08:42:09 EST
Created attachment 240454 [details]
The patch

The no-run option argument can have only one of the following values:
"caa" The test executes up to connect to AUT Agent step (inclusive)
"cdb" The test executes up to connect to database step (inclusive)
"lp" The test executes up to project load step (inclusive)
"cc" The test executes up to check test completeness step (inclusive)
"sa" The test executes up to start AUT step (inclusive)
"pte" The test executes up to prepare test execution step (inclusive)
"ca" The test executes up to connect to AUT step (inclusive)
"rpv" The test executes up to resolve predefined variables step (inclusive)
"bt" The test executes up to build test execution tree step (inclusive)
The no-run option argument is optional, with no argument the test executes up to check test completeness step (inclusive).

The patch is attached.
Comment 1 Maissmaallsmyss Maulhs-Vvuillss CLA 2014-04-25 06:51:31 EDT
Created attachment 242326 [details]
The patch for norun extention

I´ve reattached the file because the previous one was too old to apply and to review it.
Comment 2 Maissmaallsmyss Maulhs-Vvuillss CLA 2014-04-25 07:06:14 EDT
Created attachment 242327 [details]
The patch for norun extention
Comment 3 Marvin Mueller CLA 2014-04-28 04:18:48 EDT
thanks for the contribution.

In the IOConstants and NetUtil you have added methods and information for the no-run option, this is not a good place for these methods since the no run has nothing to do with Network(NetUtil) and also i have my doubt if the IOConstants since this is for input output constans. Please change this to either own classes or appropriate other classes.

Simplify the use of your NetUtil.noRunExecutionIsOverCheck() in the points where the test execution ends. Since if you call it you are also calling endTestExecution it is better to get rid of double code in using another method for it. Maybe it is also useful to have the target of the noRun option stored into a singleton so you can reuse this in the noRunExecutionIsOverCheck() in a easy way and you must not give over this as a parameter each time. This is useful since the noRun option does not change during runtime.

Also you are violating our Checkstyle in the executeJob method in the ExecutionController class. The method is to long.
Comment 4 Maissmaallsmyss Maulhs-Vvuillss CLA 2014-04-29 12:05:06 EDT
Created attachment 242502 [details]
Changed patch

Thanks  for valuable feedback! I´ve moved constants from IOConstants to newly created TestexecConstants class (org.eclipse.jubula.tools/src/org/eclipse/jubula/tools/constants/TestexecConstants.java) because we did not have any other corresponding class.

Also I´ve moved isNoRunModeValid method from NetUtils to the JobConfiguration  class because it is used only within this class and removed noRunExecutionIsOverCheck() from NetUtils.

As far as I can see, currently other testexec arguments are also passed as parameters to startTestJob and startTestSuite methods and they are not stored into a singleton though they also do not change during the runtime.
So I´ve just simplified the noRunExecutionIsOverCheck() and have not created a singlton.
Comment 5 Maissmaallsmyss Maulhs-Vvuillss CLA 2014-04-30 08:08:58 EDT
Created attachment 242536 [details]
patch with merged changes after other testexec patch apply

I´ve merged changes after other testexec patch apply. This patch is applicable
Comment 7 Maissmaallsmyss Maulhs-Vvuillss CLA 2014-05-05 08:01:17 EDT
Created attachment 242703 [details]
Patch for handling NPE caused by changes in no-run option

Change in no run option caused NPE during the test execution via the ITE. The attached patch handle this problem
Comment 8 Sebastian Struckmann CLA 2014-05-05 10:12:44 EDT
The fix looks good. I added it with: http://git.eclipse.org/c/jubula/org.eclipse.jubula.core.git/commit/?id=da13f0d4e28ee53f26c8c30112f17977eecfe109
Comment 9 Oliver Goetz CLA 2014-05-08 03:56:12 EDT
This is not fixed (see task JUB-238)
Comment 10 Sebastian Struckmann CLA 2014-05-23 03:06:45 EDT
Patch has been reapplied [1] with some small changes after temporary revert. Now, this should finally work as expected.

[1] http://git.eclipse.org/c/jubula/org.eclipse.jubula.core.git/commit/?id=931ab65dbe6ab340bd77dcae2b06ce7a315a3562
Comment 11 Oliver Goetz CLA 2014-05-28 09:50:26 EDT
This ticket is blocked by 436061
Comment 12 Oliver Goetz CLA 2014-06-05 08:52:00 EDT
All blocking tickets have been fixed, Jira task is set to done.
Comment 13 Oliver Goetz CLA 2014-06-05 08:52:26 EDT
Closed due to comment 12.