Merge lp:~eeejay/mago/dry_run into lp:~mago-contributors/mago/mago-1.0

Proposed by Eitan Isaacson
Status: Rejected
Rejected by: Eitan Isaacson
Proposed branch: lp:~eeejay/mago/dry_run
Merge into: lp:~mago-contributors/mago/mago-1.0
Diff against target: None lines
To merge this branch: bzr merge lp:~eeejay/mago/dry_run
Reviewer Review Type Date Requested Status
Joker Wild (community) test Needs Information
Javier Collado (community) Needs Information
Review via email: mp+6938@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Eitan Isaacson (eeejay) wrote :

This branch does two things:
- Defines an environment variable, DT_PATH, that could be used to run tests from any working directory.
- Adds a --dry-run option that will simply log tests being run, but not actually run them.

Revision history for this message
Javier Collado (javier.collado) wrote :

Hello,

DT_PATH seems to be a useful option. Shouldn't be a good idea to add it as command-line option?

Regarding the dry_run option, despite it seems also useful, I'm not completely sure about the implementation because I think that the "dryrun" argument is passed in a deep hierarchy before being used. If I understood correctly, the option has been added just to log the name of the test cases and test suites that would be run. Then, maybe is a better option to separate the discovery part from the running part so that it isn't needed to pass the "dryrun" argument.

I'm just guessing so maybe what I'm saying it's not possible with the current code, but I would be thinking about something like:
# discovery part logs the suites and test cases found
test_suites = discover_test_suites(DT_PATH)
# running part runs the test cases for all test suites
if not dry_run:
    for test_suite in test_suites:
        test_suite.run()

Best regards,
    Javier

review: Needs Information
Revision history for this message
Jason Cozens (jason-cozens) wrote :

Hello,

I think these are good suggestions . . .

> but I would be thinking about something like:
> # discovery part logs the suites and test cases found
> test_suites = discover_test_suites(DT_PATH)

I definitely agree we need something like this. I was going to suggest an option such as --test-info that would return information about applications, test suites, test cases. Possibly with some options as to what is returned.

I think if we go down this route we would get a cleaner division between test discovery, test selection and running tests.

Cheers,
Jason.

Revision history for this message
Eitan Isaacson (eeejay) wrote :

I am not that excited of the way I implemented this either. I think the dry run option (as opposed to test discovery) option is useful, because a user could have a fairly long command that includes many suites and specific cases, then she could run a "dry run" to see that the right things will be run before actually running the tests.

I am not married to the "dry run" concept or the code, I just need a test discovery mechanism so we could integrate well with checkbox.

Revision history for this message
Javier Collado (javier.collado) wrote :

Hello,

Please take a look at the merge that I've just proposed:
https://code.launchpad.net/~javier.collado/ubuntu-desktop-testing/suite_discovery/+merge/7055

As it's mentioned in the merge proposal, there is still some work to be done in terms of the integration between the propose_suite_file function and the new discovery code, but I believe that this change is going to be beneficial.

Best regards,
    Javier

Revision history for this message
Joker Wild (lajjr-deactivatedaccount) wrote :

Nice features, but I think when it is implemented it will allow more features we are going in a good direction.
Regards,
Leo

review: Needs Information (test)

Unmerged revisions

80. By Eitan Isaacson

Made changes a bit more lightweight. Use loggin instead of printing to stdout.

79. By Eitan Isaacson

changed to DT_PATH, to avoid GDT/UDT diversion.

78. By Eitan Isaacson

Added UDT_PATH environment variable.

77. By Eitan Isaacson

Added a --dry-run option.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/ubuntu-desktop-test'
2--- bin/ubuntu-desktop-test 2009-05-05 11:10:25 +0000
3+++ bin/ubuntu-desktop-test 2009-06-01 09:03:55 +0000
4@@ -20,8 +20,10 @@
5 from time import time, gmtime, strftime
6 from shutil import move
7
8+from desktoptesting.test_suite.main import TestSuite
9+
10 # Globals
11-TESTS_SHARE = "."
12+TESTS_SHARE = os.environ.get("DT_PATH", ".")
13 TESTS_HOME = os.environ["HOME"] + "/.ubuntu-desktop-test"
14 SCREENSHOTS_SHARE = "/tmp/ldtp-screenshots"
15
16@@ -168,13 +170,14 @@
17
18 return suite_files
19
20-def run_suite_file(suite_file, log_file, cases=None):
21+def run_suite_file(suite_file, log_file, cases=None, dryrun=False):
22 conf_file = os.path.join(TESTS_SHARE, "conffile.ini")
23- runner = TestSuiteRunner(suite_file)
24+ runner = TestSuiteRunner(suite_file, dryrun=dryrun)
25 results = runner.run(cases=cases)
26- f = open(log_file, 'w')
27- f.write(results)
28- f.close()
29+ if not dryrun:
30+ f = open(log_file, 'w')
31+ f.write(results)
32+ f.close()
33
34 def convert_log_file(log_file):
35
36@@ -208,7 +211,7 @@
37 % (html_file, xsl_file, log_file)
38 safe_run_command(command)
39
40-def process_suite_file(suite_file, target_directory, cases=None):
41+def process_suite_file(suite_file, target_directory, cases=None, dryrun=False):
42 application_name = os.path.basename(os.path.dirname(suite_file))
43 application_target = os.path.join(target_directory, application_name)
44 safe_make_directory(application_target)
45@@ -216,7 +219,7 @@
46 suite_name = os.path.basename(suite_file)
47 log_file = os.path.join(application_target,
48 suite_name.replace(".xml", ".log"))
49- run_suite_file(suite_file, log_file, cases)
50+ run_suite_file(suite_file, log_file, cases, dryrun)
51
52 convert_log_file(log_file)
53
54@@ -261,6 +264,11 @@
55 default=None,
56 help="Test cases to run (all, if not specified).")
57
58+ parser.add_option("--dry-run",
59+ action="store_true",
60+ default=False,
61+ help="Dry run, just print suites and cases.")
62+
63 (options, args) = parser.parse_args(args[1:])
64
65 # Set logging early
66@@ -300,7 +308,8 @@
67
68 # Run filtered suite file
69 for suite_file in suite_files:
70- process_suite_file(suite_file, options.target, options.case)
71+ process_suite_file(
72+ suite_file, options.target, options.case, options.dry_run)
73
74
75 return 0
76@@ -349,20 +358,24 @@
77
78
79 class TestCaseRunner(TestRunner):
80- def __init__(self, obj, xml_node):
81+ def __init__(self, obj, xml_node, dryrun=False):
82 TestRunner.__init__(self)
83+ self.dryrun = dryrun
84 self.xml_node = xml_node
85 self.name = xml_node.getAttribute('name')
86- self.test_func = getattr(
87- obj, xml_node.getElementsByTagName('method')[0].firstChild.data)
88+ method_name = \
89+ xml_node.getElementsByTagName('method')[0].firstChild.data
90+ self.test_func = getattr(obj, method_name, None)
91 args_element = xml_node.getElementsByTagName('args')
92 if args_element:
93 self.get_args(args_element[0])
94
95 def run(self, logger):
96+ logging.debug("Running case %s" % self.name)
97 starttime = time()
98 try:
99- rv = self.test_func(**self.args)
100+ if not self.dryrun:
101+ rv = self.test_func(**self.args)
102 except AssertionError, e:
103 # The test failed.
104 if len(e.args) > 1:
105@@ -399,12 +412,15 @@
106 self.set_result('time', time() - starttime)
107
108 self.add_results_to_node(self.xml_node)
109-
110+
111 class TestSuiteRunner(TestRunner):
112- def __init__(self, suite_file, loggerclass=None):
113+ _test_case_runner_cls = TestCaseRunner
114+ def __init__(self, suite_file, loggerclass=None, dryrun=False):
115 TestRunner.__init__(self)
116+ logging.debug("Loading suite file %s" % suite_file)
117 self.dom = xml.dom.minidom.parse(suite_file)
118 self._strip_whitespace(self.dom.documentElement)
119+ self.name = self.dom.documentElement.getAttribute('name')
120 clsname, modname = None, None
121 case_runners = []
122 for node in self.dom.documentElement.childNodes:
123@@ -429,12 +445,17 @@
124
125 cls = getattr(mod, clsname)
126
127- self.testsuite = cls(**self.args)
128+ if dryrun:
129+ self.testsuite = TestSuite()
130+ else:
131+ self.testsuite = cls(**self.args)
132
133 self.case_runners = \
134- [TestCaseRunner(self.testsuite, c) for c in case_runners]
135+ [self._test_case_runner_cls(self.testsuite, c, dryrun) \
136+ for c in case_runners]
137
138 def run(self, loggerclass=None, setup_once=True, cases=None):
139+ logging.debug("Running suite %s" % self.name)
140 try:
141 self._run(loggerclass, setup_once, cases)
142 except Exception, e:

Subscribers

People subscribed via source and target branches

to status/vote changes: