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

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

unspecified
Bug Flags:
in-testsuite +

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 634437 [details]
Patch

Pointer to Github pull-request
(Assignee)

Comment 2

6 years ago
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.
Attachment #634437 - Attachment description: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/50 → Patch
Attachment #634437 - Flags: review?(jhammel)
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
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

6 years ago
(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.
(Assignee)

Comment 6

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

Comment 7

6 years ago
Comment on attachment 634437 [details]
Patch

Nice and simple, I like it
Attachment #634437 - Flags: review?(jhammel) → review+
(Assignee)

Comment 8

6 years ago
https://github.com/mozautomation/mozmill/commit/dd78957442edd1402182a19818e4254069ca85d3
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.