766113 - Command line options -t and -m should exclude each other

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Right now you are able to specify both the -t (--test) and -m (--manifest) together at the command line. That results in totally unexpected behavior and has to be avoided. If you want to specify tests use -t or -m but not both.

Mozmill should make sure that both options can't be specified.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Patch

Pointer to Github pull-request

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 634437 [details]
Patch

Right now without a test but before I write test I want to know what you think about the patch.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Jeff, I have updated the patch and added a test for. It's not the safest right now but once I have moved all the tests and manifest related code into MozMill base, we can handle that inside the run_tests method.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So I figured that this behavior is on purpose?

https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/__init__.py#L529

Do we really want that? Where will this test be added? Always at the end? I still think we should drop that. But if you agree here, I would have to update my patch once more and get this code removed.

Comment 5

[Jeff Hammel]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> So I figured that this behavior is on purpose?
> 
> https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/
> __init__.py#L529
> 
> Do we really want that? Where will this test be added? Always at the end? I
> still think we should drop that. But if you agree here, I would have to
> update my patch once more and get this code removed.

It is on purpose.  And yes, it gets added to the end.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So why do we want to have this behavior? What does it buy us beside confusion what gets run and when?

Comment 7

[Jeff Hammel]

Comment on attachment 634437 [details]
Patch

Nice and simple, I like it

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

https://github.com/mozautomation/mozmill/commit/dd78957442edd1402182a19818e4254069ca85d3