Bug 82333 – [GTK] Use gtester -s to skip individual test cases instead of unit tests as a whole

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

It requires a glib bump, so it can't be landed for now. Patch attached to bug #41630 bumps glib requirements, so we can fix it when that one lands.

[Carlos Garcia Campos]

Created attachment 134050 [details]
Patch

[Martin Robinson]

Comment on attachment 134050 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=134050&action=review

Looks good, but consider the small changes below before landing.

> Tools/Scripts/run-gtk-tests:34
> +    class SkippedTest:
> +
> +        def __init__(self, test, reason, bug=None, test_cases=[]):
> +            self.test = test

Might as well make this a top-level class. There's only one class in this file, but as time goes on we'll probably add more and want them to interact.

> Tools/Scripts/run-gtk-tests:230
> +        self._tests = [test for test in self._tests if self._should_run_test(test)]

You could write this as: self._tests = itertools.ifilterfalse(self._should_run_test, self._tests)

> Tools/Scripts/run-gtk-tests:248
> -            names = [os.path.basename(test) for test in failed_tests]
> +            names = [test.lstrip(self._programs_path) for test in failed_tests]

This seems unrelated -- there's a lot of general cleanup in this patch though.

> Tools/Scripts/run-gtk-tests:261
> +            for skipped in self._skipped_tests:
> +                sys.stdout.write("%s" % skipped.test)
> +                if skipped.test_cases:
> +                    sys.stdout.write(" [%s]" % ", ".join(skipped.test_cases))
> +                sys.stdout.write(": %s " % skipped.reason)
> +                if skipped.bug is not None:
> +                    sys.stdout.write("(https://bugs.webkit.org/show_bug.cgi?id=%d)" % skipped.bug)
> +                sys.stdout.write("\n")

I think it'd make sense to have a SkippedTest.__str__ or SkippedTest.__repr__ that converted the class to a string. Then you would just do sys.stdout.write("%s" % skipped)

[Carlos Garcia Campos]

(In reply to comment #2)
> (From update of attachment 134050 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134050&action=review
> 
> Looks good, but consider the small changes below before landing.
> 
> > Tools/Scripts/run-gtk-tests:34
> > +    class SkippedTest:
> > +
> > +        def __init__(self, test, reason, bug=None, test_cases=[]):
> > +            self.test = test
> 
> Might as well make this a top-level class. There's only one class in this file, but as time goes on we'll probably add more and want them to interact.

Ok.
 
> > Tools/Scripts/run-gtk-tests:261
> > +            for skipped in self._skipped_tests:
> > +                sys.stdout.write("%s" % skipped.test)
> > +                if skipped.test_cases:
> > +                    sys.stdout.write(" [%s]" % ", ".join(skipped.test_cases))
> > +                sys.stdout.write(": %s " % skipped.reason)
> > +                if skipped.bug is not None:
> > +                    sys.stdout.write("(https://bugs.webkit.org/show_bug.cgi?id=%d)" % skipped.bug)
> > +                sys.stdout.write("\n")
> 
> I think it'd make sense to have a SkippedTest.__str__ or SkippedTest.__repr__ that converted the class to a string. Then you would just do sys.stdout.write("%s" % skipped)

Makes a lot of sense, I'll do it

[Eric Seidel (no email)]

Attachment 134050 [details] was posted by a committer and has review+, assigning to Carlos Garcia Campos for commit.

[Carlos Garcia Campos]

Committed r112404: <http://trac.webkit.org/changeset/112404>