Closed Bug 776946 Opened 12 years ago Closed 9 years ago

PerfConfigurator should provide an informative error message if --activeTests is given a bad argument

Categories

(Testing :: Talos, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jwatt, Assigned: parkouss)

References

Details

Attachments

(1 file)

PerfConfigurator should provide an informative error message if --activeTests is given a bad argument. Currently it gives a very unhelpful assertion.
Attached patch 776946.patchSplinter Review
With this patch:

> talos --cycles 2 -a resize -e `which firefox` --develop
> ERROR: No definition found for test(s): ['resize']

this works fine locally if I give a right syntax.
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8643688 - Flags: review?(jmaher)
Comment on attachment 8643688 [details] [diff] [review]
776946.patch

Review of attachment 8643688 [details] [diff] [review]:
-----------------------------------------------------------------

one question here, seems a bit critical.  Ask for review again if it isn't an issue.

::: talos/run_tests.py
@@ +300,4 @@
>      except ConfigurationError, exc:
>          sys.exit("ERROR: %s" % exc)
> +    utils.startLogger('debug' if config['debug'] else 'info')
> +    sys.exit(run_tests(config, browser_config))

do we not need a try clause around run_tests?
Attachment #8643688 - Flags: review?(jmaher) → review-
No. ConfigurationError is only raised from config.py. This was a mistake of my patch with config.py.


$ grep 'Configu' `find talos -name '*.py'`
talos/config.py:class ConfigurationError(Exception):
talos/config.py:            raise ConfigurationError(
talos/config.py:            raise ConfigurationError(
talos/config.py:        raise ConfigurationError("No definition found for test(s): %s"
talos/run_tests.py:from config import get_configs, ConfigurationError
talos/run_tests.py:    except ConfigurationError, exc:
talos/startup_test/media/media_manager.py:# Handler for Server Configuration Commands


Hmm, maybe you could just switch to r+ on the same patch if you agree ?
Comment on attachment 8643688 [details] [diff] [review]
776946.patch

Review of attachment 8643688 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for the explanation.
Attachment #8643688 - Flags: review- → review+
Landed in https://hg.mozilla.org/build/talos/rev/49bcbbb8c263
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: