Merge lp:~hikiko/mir/mir.1238000 into lp:mir

[Eleni Maria Stea]

Test GBMBufferAllocatorTest.bypass_disables_via_environment overrides the MIR_BYPASS env variable, causing other tests that use the MIR_BYPASS to fail when we run the unit-tests with --gtest_repeat=N, N>1. Set back the MIR_BYPASS env. var.

Test GBMBufferAllocatorTest.bypass_disables_via_environment overrides the MIR_BYPASS env variable, causing other tests that use the MIR_BYPASS to fail when we run the unit-tests with --gtest_repeat=N, N>1. Set back the MIR_BYPASS env. var.

[Alexandros Frantzis (afrantzis)]

8 + setenv("MIR_BYPASS", "1", 1);

This also changes the test environment. The tests implicitly assume that MIR_BYPASS is unset, so let's make the assumption explicit by unsetting it for every test in SetUp.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1131
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/158/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2342
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2227
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/155
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/155/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/158/rebuild

review: Approve (continuous-integration)

[Eleni Maria Stea (hikiko)]

I unset it in both places because if a test runs after the bypass_disables_via_environment unit test but before the new setup, it might fail.

[Alexandros Frantzis (afrantzis)]

> I unset it in both places because if a test runs after the bypass_disables_via_environment unit
> test but before the new setup, it might fail.

I don't think that's needed; SetUp() is run before every test is executed. From the googletest FAQ:

"The first thing to remember is that Google Test does not reuse the same test fixture object across multiple tests. For each TEST_F, Google Test will create a fresh test fixture object, immediately call SetUp(), run the test, call TearDown(), and then immediately delete the test fixture object."

[Kevin DuBois (kdub)]

yes, lets rm ln 16. looks okay after that (I'll pre-approve, based on removing the second unsetenv)

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1133
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/161/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2347
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2232
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/158
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/158/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/mir-team-mir-development-branch-ci/161/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> I unset it in both places because if a test runs after the bypass_disables_ via_environment unit test but before the new setup, it might fail.

Actually, on second thought, you make a valid point here since we are allowed to run tests (including ones from other test cases) in an arbitrary order, although we don't do so normally. In light of this, let's just keep the second unsetenv (end of the test). Ideally that should happen automatically when we leave the test function scope (using a wrapper class to set/unset the env, ala RAII), to ensure unsetenv gets called even in case an ASSERT fails.

[Daniel van Vugt (vanvugt)]

I like keeping both unsets.

review: Approve

=== modified file 'tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp'
--- tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp	2013-09-18 14:41:59 +0000
+++ tests/unit-tests/graphics/gbm/test_gbm_buffer_allocator.cpp	2013-10-14 13:41:47 +0000
@@ -51,6 +51,7 @@
     virtual void SetUp()
     {
         using namespace testing;
+        unsetenv("MIR_BYPASS");
 
         fake_devices.add_standard_drm_devices();
 
@@ -154,6 +155,7 @@
     auto buf = alloc.alloc_buffer(properties);
     ASSERT_TRUE(buf.get() != NULL);
     EXPECT_FALSE(buf->can_bypass());
+    unsetenv("MIR_BYPASS");
 }
 
 TEST_F(GBMBufferAllocatorTest, correct_buffer_format_translation_argb_8888)