Bug 52863 – nrwt: suppress remaining integration tests for now

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

nrwt: suppress remaining integration tests for now

[Dirk Pranke]

Created attachment 79687 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 79687 [details]
Patch

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

Maybe we should think about creating a new file suffix _integrationtest.py or something and having test-webkitpy know how to run them with passed --integration or some-such?

This looks good except for the None to [] change.

> Tools/Scripts/webkitpy/layout_tests/layout_package/printing_unittest.py:115
> +    def get_printer(self, args=[], single_threaded=False,

None is preferred for default arguments.  The default values are shared by all instances, so if anything ever tried to mutate that args value, we'd have bad news bears. :)

[Dirk Pranke]

cc'ing some other people in case they have thoughts on this ...

1) regarding integration tests, my thinking had been to modify test-webkitpy to support some sort of @integration decorator that could mark up tests or classes. I would prefer to keep things in a single file because I think that allows for much better reuse of fixtures in between tests.

2) regarding using None instead of [] as a default argument value, Mihai had suggested in another code review a few days ago that [] might be preferable to None in functions where you weren't modifying the argument inside the function (the advantage is that it's clearer and shorter). That said, it's certainly more fragile. Arguably there's a middle ground where it might be safe to use [] if the variable is never passed to another function (so at least it's visible and in your control), but after having thought about it more, I think I'm inclined to agree with you that we should just use None to be safe.

Anyone else have any thoughts on either point?

[Dirk Pranke]

Committed r76398: <http://trac.webkit.org/changeset/76398>

[Mihai Parparita]

(In reply to comment #3)
> 2) regarding using None instead of [] as a default argument value, Mihai had suggested in another code review a few days ago that [] might be preferable to None in functions where you weren't modifying the argument inside the function (the advantage is that it's clearer and shorter). That said, it's certainly more fragile. Arguably there's a middle ground where it might be safe to use [] if the variable is never passed to another function (so at least it's visible and in your control), but after having thought about it more, I think I'm inclined to agree with you that we should just use None to be safe.

I don't actually have a strong preference about this, so if you want to always use None (to be safe, in case the function is changed to modify the argument later), I can promise to point it out in reviews :)

[Mihai Parparita]

(In reply to comment #5)
> I can promise to point it out in reviews :)

Err, _not_ point it out in reviews.

[Dirk Pranke]

*** Bug 46322 has been marked as a duplicate of this bug. ***