Bug 49361 – add better unit tests for bug 49360 (nrwt respecting set-webkit-configuration)

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Dirk Pranke]

add better unit tests for bug 49360 (nrwt respecting set-webkit-configuration)

[Dirk Pranke]

Created attachment 73573 [details]
Patch

[Dirk Pranke]

So, I believe that it is impossible to write a test that actually catches the failure and yet remains a standalone test that can be run as part of a larger test suite. In order to remain standalone, the test needs to call clear_cached_configuration(), but calling that resets the value of _determined_configuration, meaning that you can't reproduce the failure.

At any rate, you can reproduce the failure by running the tests separately and commenting out the clear_cached_configuration() call in IntTest.setUp(), and then apply the patch in 49360 and verify that it is fixed.

This patch introduces the concept of "integration tests" that actually test the real code paths (running set-webkit-configuration and then calling config.default_configuration()). These tests are disabled by default, because they will mess with the state of the current checkout.

We don't currently have a good way of providing an isolated build/checkout for tests. However, we should still be able to run these tests on demand, or in configurations where we don't worry too much about perturbing the checkout state (e.g., on the bots, where we do clean builds if something fails).

Ideally we can modify test-webkitpy to support different flags for the types of tests we want, and can use python decorators to better classify the test methods as to whether they are "safe" or "unsafe" (or slow, or require a real HTTP server, etc.). I have filed a separate bug for that effort (49299).

In the meantime, I wanted to write tests that would be the same tests run with either real objects or mock objects (i.e., without cutting and pasting the code). After several attempts at this, the submitted patch is the best I could come up with that also allowed us to control whether or not to run unsafe versions.

I am open to any better ideas of how to write these tests.

[Adam Roben (:aroben)]

(In reply to comment #2)
> So, I believe that it is impossible to write a test that actually catches the failure and yet remains a standalone test that can be run as part of a larger test suite. In order to remain standalone, the test needs to call clear_cached_configuration(), but calling that resets the value of _determined_configuration, meaning that you can't reproduce the failure.

Could you have the test invoke a separate python process that would just call default_configuration and print out the result?

[Dirk Pranke]

(In reply to comment #3)
> (In reply to comment #2)
> > So, I believe that it is impossible to write a test that actually catches the failure and yet remains a standalone test that can be run as part of a larger test suite. In order to remain standalone, the test needs to call clear_cached_configuration(), but calling that resets the value of _determined_configuration, meaning that you can't reproduce the failure.
> 
> Could you have the test invoke a separate python process that would just call default_configuration and print out the result?

Ugh. Yes, that would work. I hate to have a separate script just for this, but given that it's broken twice it's probably worth it until I can get rid of the global cache and make this unecessary.

[Dirk Pranke]

Comment on attachment 73573 [details]
Patch

cancelling review flag. Per aroben's suggestion, I've added a unit test that reproduces the failure. I'll put this whole "integration tests" concept on hold for now until we have the support in test-webkitpy, and then add some integration tests back in.