Closed Bug 633997 Opened 10 years ago Closed 10 years ago

[manifests] add API to filter by OS, toolkit, etc.

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: k0scist, Assigned: k0scist)

Details

Attachments

(1 file, 1 obsolete file)

the integration layer, TestManifest, should be flushed out to actionably filter tests: e.g. by os, toolkit, etc.
proposal for elementary filtering
Assignee: nobody → jhammel
Status: NEW → ASSIGNED
Attachment #512191 - Flags: review?(jmaher)
Comment on attachment 512191 [details] [diff] [review]
filter on os, toolkit, and exists

>-        # TODO: could filter out by current platform, existence, etc
>+        # ignore tests that do not exist
>+        if exists:
>+            tests = [i for i in tests if os.path.exists(test['path'])]
>+
>+        # filter by tags
>+        for tag in ('os', 'toolkit'):

can we make this list more configurable?


>+            value = locals()[tag]
>+            if value:
>+                tests = self.filter(tag, value, tests)
>+

why do we check for value?  what about os.ignore?  Maybe I don't understand what locals() and value are.


r=me with at least the list of tags being more configurable (self.defaulttags or something like that).
Attachment #512191 - Flags: review?(jmaher) → review+
> +	  # filter by tags
> >+	  for tag in ('os', 'toolkit'):
> can we make this list more configurable?

this incarnation uses the keyword args as tags to filter by.  its probably a bit cleaner, just makes it less clear from function argspec what the available tags are
Attachment #512191 - Attachment is obsolete: true
Attachment #512217 - Flags: review?(jmaher)
Comment on attachment 512217 [details] [diff] [review]
filter on os, toolkit, and exists

looks good.  Can we file a followup bug for some unittests that exercise this well?
Attachment #512217 - Flags: review?(jmaher) → review+
pushed as http://hg.mozilla.org/automation/ManifestDestiny/rev/e0068279aac8 ; there are some minimal doctests in the patch; does that suffice?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.