Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6ced2e42 authored by Yu Shan's avatar Yu Shan
Browse files

Fix flakiness in DefaultVehicleHalTest. 

Test: atest DefaultVehicleHalTest
Bug: 270225041
Change-Id: I1657ad038aac9e6a153aa458c13bd0720be23f1f
parent 6d065549

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

+29 −32
Original line number Diff line number Diff line
@@ -671,8 +671,8 @@ TEST_F(DefaultVehicleHalTest, testGetValuesNoReadPermission) { 
}



TEST_F(DefaultVehicleHalTest, testGetValuesFinishBeforeTimeout) {

    // timeout: 0.1s

    int64_t timeout = 100000000;

    // timeout: 1s

    int64_t timeout = 1000000000;

    setTimeout(timeout);



    GetValueRequests requests;
@@ -681,17 +681,15 @@ TEST_F(DefaultVehicleHalTest, testGetValuesFinishBeforeTimeout) { 


    ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());



    // The response would be returned after 0.05s.

    getHardware()->setSleepTime(timeout / 2);

    // The response would be returned after 0.01s.

    getHardware()->setSleepTime(timeout / 100);

    getHardware()->addGetValueResponses(expectedResults);



    auto status = getClient()->getValues(getCallbackClient(), requests);



    ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();



    // Wait for the response.

    std::this_thread::sleep_for(std::chrono::nanoseconds(timeout));



    ASSERT_TRUE(getCallback()->waitForGetValueResults(1, timeout)) << "no results in callback";

    auto maybeGetValueResults = getCallback()->nextGetValueResults();

    ASSERT_TRUE(maybeGetValueResults.has_value()) << "no results in callback";

    EXPECT_EQ(maybeGetValueResults.value().payloads, expectedResults) << "results mismatch";
@@ -699,8 +697,8 @@ TEST_F(DefaultVehicleHalTest, testGetValuesFinishBeforeTimeout) { 
}



TEST_F(DefaultVehicleHalTest, testGetValuesFinishAfterTimeout) {

    // timeout: 0.1s

    int64_t timeout = 100000000;

    // timeout: 0.01s

    int64_t timeout = 10000000;

    setTimeout(timeout);



    GetValueRequests requests;
@@ -709,17 +707,14 @@ TEST_F(DefaultVehicleHalTest, testGetValuesFinishAfterTimeout) { 


    ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());



    // The response would be returned after 0.2s.

    getHardware()->setSleepTime(timeout * 2);

    // The response would be returned after 0.1s.

    getHardware()->setSleepTime(timeout * 10);

    getHardware()->addGetValueResponses(expectedResults);



    auto status = getClient()->getValues(getCallbackClient(), requests);



    ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();



    // Wait for the response.

    std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));



    for (size_t i = 0; i < expectedResults.size(); i++) {

        expectedResults[i] = {

                .requestId = expectedResults[i].requestId,
@@ -728,6 +723,8 @@ TEST_F(DefaultVehicleHalTest, testGetValuesFinishAfterTimeout) { 
        };

    }



    ASSERT_TRUE(getCallback()->waitForGetValueResults(1, timeout * 100))

            << "no results in callback";

    auto maybeGetValueResults = getCallback()->nextGetValueResults();

    ASSERT_TRUE(maybeGetValueResults.has_value()) << "no results in callback";

    ASSERT_THAT(maybeGetValueResults.value().payloads, UnorderedElementsAreArray(expectedResults))
@@ -960,8 +957,8 @@ TEST_P(SetValuesInvalidRequestTest, testSetValuesInvalidRequest) { 
}



TEST_F(DefaultVehicleHalTest, testSetValuesFinishBeforeTimeout) {

    // timeout: 0.1s

    int64_t timeout = 100000000;

    // timeout: 1s

    int64_t timeout = 1000000000;

    setTimeout(timeout);



    SetValueRequests requests;
@@ -970,17 +967,15 @@ TEST_F(DefaultVehicleHalTest, testSetValuesFinishBeforeTimeout) { 


    ASSERT_TRUE(setValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());



    // The response would be returned after 0.05s.

    getHardware()->setSleepTime(timeout / 2);

    // The response would be returned after 0.01s.

    getHardware()->setSleepTime(timeout / 100);

    getHardware()->addSetValueResponses(expectedResults);



    auto status = getClient()->setValues(getCallbackClient(), requests);



    ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();



    // Wait for the response.

    std::this_thread::sleep_for(std::chrono::nanoseconds(timeout));



    ASSERT_TRUE(getCallback()->waitForSetValueResults(1, timeout)) << "no set value results";

    auto maybeSetValueResults = getCallback()->nextSetValueResults();

    ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";

    EXPECT_EQ(maybeSetValueResults.value().payloads, expectedResults) << "results mismatch";
@@ -988,8 +983,8 @@ TEST_F(DefaultVehicleHalTest, testSetValuesFinishBeforeTimeout) { 
}



TEST_F(DefaultVehicleHalTest, testSetValuesFinishAfterTimeout) {

    // timeout: 0.1s

    int64_t timeout = 100000000;

    // timeout: 0.01s

    int64_t timeout = 10000000;

    setTimeout(timeout);



    SetValueRequests requests;
@@ -998,17 +993,14 @@ TEST_F(DefaultVehicleHalTest, testSetValuesFinishAfterTimeout) { 


    ASSERT_TRUE(setValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());



    // The response would be returned after 0.2s.

    getHardware()->setSleepTime(timeout * 2);

    // The response would be returned after 0.1s.

    getHardware()->setSleepTime(timeout * 10);

    getHardware()->addSetValueResponses(expectedResults);



    auto status = getClient()->setValues(getCallbackClient(), requests);



    ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();



    // Wait for the response.

    std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));



    for (size_t i = 0; i < expectedResults.size(); i++) {

        expectedResults[i] = {

                .requestId = expectedResults[i].requestId,
@@ -1016,6 +1008,7 @@ TEST_F(DefaultVehicleHalTest, testSetValuesFinishAfterTimeout) { 
        };

    }



    ASSERT_TRUE(getCallback()->waitForSetValueResults(1, timeout * 100)) << "no set value results";

    auto maybeSetValueResults = getCallback()->nextSetValueResults();

    ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";

    ASSERT_THAT(maybeSetValueResults.value().payloads, UnorderedElementsAreArray(expectedResults))
@@ -1456,7 +1449,7 @@ TEST_F(DefaultVehicleHalTest, testUnsubscribeContinuous) { 
    std::vector<SubscribeOptions> options = {

            {

                    .propId = GLOBAL_CONTINUOUS_PROP,

                    .sampleRate = 20.0,

                    .sampleRate = 100.0,

            },

    };
@@ -1469,16 +1462,20 @@ TEST_F(DefaultVehicleHalTest, testUnsubscribeContinuous) { 


    ASSERT_TRUE(status.isOk()) << "unsubscribe failed: " << status.getMessage();



    // Wait for the last events to come.

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    // Clear existing events.

    while (getCallback()->nextOnPropertyEventResults().has_value()) {

        // Do nothing.

    }



    // Wait for a while, make sure no new events are generated.

    // Wait for a while, make sure no new events are generated. If still subscribed, this should

    // generate around 10 events.

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    ASSERT_FALSE(getCallback()->nextOnPropertyEventResults().has_value())

            << "No property event should be generated after unsubscription";

    ASSERT_EQ(getCallback()->countOnPropertyEventResults(), 0u)

            << "Property event generation must stop after unsubscription";

}



class SubscribeInvalidOptionsTest

automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.cpp

+42 −8
Original line number Diff line number Diff line
@@ -16,6 +16,9 @@ 


#include "MockVehicleCallback.h"



#include <android-base/thread_annotations.h>

#include <chrono>



namespace android {

namespace hardware {

namespace automotive {
@@ -27,6 +30,7 @@ using ::aidl::android::hardware::automotive::vehicle::GetValueResults; 
using ::aidl::android::hardware::automotive::vehicle::SetValueResults;

using ::aidl::android::hardware::automotive::vehicle::VehiclePropErrors;

using ::aidl::android::hardware::automotive::vehicle::VehiclePropValues;

using ::android::base::ScopedLockAssertion;

using ::ndk::ScopedAStatus;

using ::ndk::ScopedFileDescriptor;
@@ -46,21 +50,35 @@ static ScopedAStatus storeResults(const T& results, std::list<T>* storedResults) 
}  // namespace



ScopedAStatus MockVehicleCallback::onGetValues(const GetValueResults& results) {

    ScopedAStatus result;

    {

        std::scoped_lock<std::mutex> lockGuard(mLock);

    return storeResults(results, &mGetValueResults);

        result = storeResults(results, &mGetValueResults);

    }

    mCond.notify_all();

    return result;

}



ScopedAStatus MockVehicleCallback::onSetValues(const SetValueResults& results) {

    ScopedAStatus result;

    {

        std::scoped_lock<std::mutex> lockGuard(mLock);

    return storeResults(results, &mSetValueResults);

        result = storeResults(results, &mSetValueResults);

    }

    mCond.notify_all();

    return result;

}



ScopedAStatus MockVehicleCallback::onPropertyEvent(const VehiclePropValues& results,

                                                   int32_t sharedMemoryFileCount) {

    ScopedAStatus result;

    {

        std::scoped_lock<std::mutex> lockGuard(mLock);



        mSharedMemoryFileCount = sharedMemoryFileCount;

    return storeResults(results, &mOnPropertyEventResults);

        result = storeResults(results, &mOnPropertyEventResults);

    }

    mCond.notify_all();

    return result;

}



ScopedAStatus MockVehicleCallback::onPropertySetError(const VehiclePropErrors&) {
@@ -87,6 +105,22 @@ size_t MockVehicleCallback::countOnPropertyEventResults() { 
    return mOnPropertyEventResults.size();

}



bool MockVehicleCallback::waitForSetValueResults(size_t size, size_t timeoutInNano) {

    std::unique_lock lk(mLock);

    return mCond.wait_for(lk, std::chrono::nanoseconds(timeoutInNano), [this, size] {

        ScopedLockAssertion lockAssertion(mLock);

        return mSetValueResults.size() >= size;

    });

}



bool MockVehicleCallback::waitForGetValueResults(size_t size, size_t timeoutInNano) {

    std::unique_lock lk(mLock);

    return mCond.wait_for(lk, std::chrono::nanoseconds(timeoutInNano), [this, size] {

        ScopedLockAssertion lockAssertion(mLock);

        return mGetValueResults.size() >= size;

    });

}



}  // namespace vehicle

}  // namespace automotive

}  // namespace hardware

automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.h

+4 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ 
#include <aidl/android/hardware/automotive/vehicle/BnVehicleCallback.h>

#include <android-base/thread_annotations.h>



#include <condition_variable>

#include <list>

#include <mutex>

#include <optional>
@@ -63,9 +64,12 @@ class MockVehicleCallback final 
    std::optional<aidl::android::hardware::automotive::vehicle::VehiclePropValues>

    nextOnPropertyEventResults();

    size_t countOnPropertyEventResults();

    bool waitForSetValueResults(size_t size, size_t timeoutInNano);

    bool waitForGetValueResults(size_t size, size_t timeoutInNano);



  private:

    std::mutex mLock;

    std::condition_variable mCond;

    std::list<aidl::android::hardware::automotive::vehicle::GetValueResults> mGetValueResults

            GUARDED_BY(mLock);

    std::list<aidl::android::hardware::automotive::vehicle::SetValueResults> mSetValueResults

automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp

+11 −3
Original line number Diff line number Diff line
@@ -231,7 +231,7 @@ TEST_F(SubscriptionManagerTest, testUnsubscribeGlobalContinuous) { 
    std::vector<SubscribeOptions> options = {{

            .propId = 0,

            .areaIds = {0},

            .sampleRate = 10.0,

            .sampleRate = 100.0,

    }};



    auto result = getManager()->subscribe(getCallbackClient(), options, true);
@@ -240,11 +240,13 @@ TEST_F(SubscriptionManagerTest, testUnsubscribeGlobalContinuous) { 
    result = getManager()->unsubscribe(getCallbackClient()->asBinder().get());

    ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();



    // Wait for the last events to come.

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    clearEvents();



    std::this_thread::sleep_for(std::chrono::milliseconds(200));

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    // Theoretically trigger 10 times, but check for at least 9 times to be stable.

    ASSERT_TRUE(getEvents().empty());

}
@@ -269,6 +271,9 @@ TEST_F(SubscriptionManagerTest, testUnsubscribeMultipleAreas) { 
                                       std::vector<int32_t>({0}));

    ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();



    // Wait for the last events to come.

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    clearEvents();



    std::this_thread::sleep_for(std::chrono::seconds(1));
@@ -301,6 +306,9 @@ TEST_F(SubscriptionManagerTest, testUnsubscribeByCallback) { 
    result = getManager()->unsubscribe(getCallbackClient()->asBinder().get());

    ASSERT_TRUE(result.ok()) << "failed to unsubscribe: " << result.error().message();



    // Wait for the last events to come.

    std::this_thread::sleep_for(std::chrono::milliseconds(100));



    clearEvents();



    std::this_thread::sleep_for(std::chrono::seconds(1));