From f4fe814729466d91764eaa8a88905e71e5fe94b6 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Thu, 7 Jan 2021 16:15:27 -0800 Subject: [PATCH 1/2] fix ax unique id flake --- .../accessibility/ax/platform/ax_unique_id.cc | 2 +- .../ax/platform/ax_unique_id_unittest.cc | 37 ++++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/third_party/accessibility/ax/platform/ax_unique_id.cc b/third_party/accessibility/ax/platform/ax_unique_id.cc index 75e318419d70c..ed6de01d1e82b 100644 --- a/third_party/accessibility/ax/platform/ax_unique_id.cc +++ b/third_party/accessibility/ax/platform/ax_unique_id.cc @@ -44,7 +44,7 @@ int32_t AXUniqueId::GetNextAXUniqueId(const int32_t max_id) { const int32_t prev_id = current_id; do { - if (current_id == max_id) { + if (current_id >= max_id) { current_id = 1; has_wrapped = true; } else { diff --git a/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc b/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc index 72f7d9d2823e9..6e57a9d6bfb1a 100644 --- a/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc +++ b/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc @@ -28,14 +28,23 @@ class AXTestSmallBankUniqueId : public AXUniqueId { BASE_DISALLOW_COPY_AND_ASSIGN(AXTestSmallBankUniqueId); }; +class AXTestBigBankUniqueId : public AXUniqueId { + public: + AXTestBigBankUniqueId(); + ~AXTestBigBankUniqueId() override; + + private: + friend class AXUniqueId; + BASE_DISALLOW_COPY_AND_ASSIGN(AXTestBigBankUniqueId); +}; + AXTestSmallBankUniqueId::AXTestSmallBankUniqueId() : AXUniqueId(kMaxId) {} AXTestSmallBankUniqueId::~AXTestSmallBankUniqueId() = default; +AXTestBigBankUniqueId::AXTestBigBankUniqueId() : AXUniqueId(kMaxId * kMaxId) {} +AXTestBigBankUniqueId::~AXTestBigBankUniqueId() = default; + TEST(AXPlatformUniqueIdTest, UnassignedIdsAreReused) { - // TODO(chunhtai): enable this test once the flake has been - // resolved. - // https://github.com/flutter/flutter/issues/73512 - GTEST_SKIP() << "Flaky Test: https://github.com/flutter/flutter/issues/73512"; // Create a bank of ids that uses up all available ids. // Then remove an id and replace with a new one. Since it's the only // slot available, the id will end up having the same value, rather than @@ -47,8 +56,7 @@ TEST(AXPlatformUniqueIdTest, UnassignedIdsAreReused) { } static int kIdToReplace = 10; - std::unique_ptr& unique = ids[kIdToReplace]; - int32_t expected_id = unique->Get(); + int32_t expected_id = ids[kIdToReplace]->Get(); // Delete one of the ids and replace with a new one. ids[kIdToReplace] = nullptr; @@ -58,4 +66,21 @@ TEST(AXPlatformUniqueIdTest, UnassignedIdsAreReused) { EXPECT_EQ(ids[kIdToReplace]->Get(), expected_id); } +TEST(AXPlatformUniqueIdTest, DoesCreateCorrectId) { + std::unique_ptr ids[kMaxId]; + // Creates and releases to fill up the internal static counter. + for (int i = 0; i < kMaxId; i++) { + ids[i] = std::make_unique(); + } + for (int i = 0; i < kMaxId; i++) { + ids[i].reset(nullptr); + } + // Creates an unique id whose max value is less than the internal + // static counter. + std::unique_ptr unique_id = + std::make_unique(); + + EXPECT_LE(unique_id->Get(), kMaxId); +} + } // namespace ui From 3872589b4290687c8eb79215cecfbd66705f8222 Mon Sep 17 00:00:00 2001 From: Chun-Heng Tai Date: Mon, 11 Jan 2021 11:30:56 -0800 Subject: [PATCH 2/2] update test --- .../ax/platform/ax_unique_id_unittest.cc | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc b/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc index 6e57a9d6bfb1a..5f62ad3d115f7 100644 --- a/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc +++ b/third_party/accessibility/ax/platform/ax_unique_id_unittest.cc @@ -28,22 +28,9 @@ class AXTestSmallBankUniqueId : public AXUniqueId { BASE_DISALLOW_COPY_AND_ASSIGN(AXTestSmallBankUniqueId); }; -class AXTestBigBankUniqueId : public AXUniqueId { - public: - AXTestBigBankUniqueId(); - ~AXTestBigBankUniqueId() override; - - private: - friend class AXUniqueId; - BASE_DISALLOW_COPY_AND_ASSIGN(AXTestBigBankUniqueId); -}; - AXTestSmallBankUniqueId::AXTestSmallBankUniqueId() : AXUniqueId(kMaxId) {} AXTestSmallBankUniqueId::~AXTestSmallBankUniqueId() = default; -AXTestBigBankUniqueId::AXTestBigBankUniqueId() : AXUniqueId(kMaxId * kMaxId) {} -AXTestBigBankUniqueId::~AXTestBigBankUniqueId() = default; - TEST(AXPlatformUniqueIdTest, UnassignedIdsAreReused) { // Create a bank of ids that uses up all available ids. // Then remove an id and replace with a new one. Since it's the only @@ -67,12 +54,13 @@ TEST(AXPlatformUniqueIdTest, UnassignedIdsAreReused) { } TEST(AXPlatformUniqueIdTest, DoesCreateCorrectId) { - std::unique_ptr ids[kMaxId]; + int kLargerThanMaxId = kMaxId * 2; + std::unique_ptr ids[kLargerThanMaxId]; // Creates and releases to fill up the internal static counter. - for (int i = 0; i < kMaxId; i++) { - ids[i] = std::make_unique(); + for (int i = 0; i < kLargerThanMaxId; i++) { + ids[i] = std::make_unique(); } - for (int i = 0; i < kMaxId; i++) { + for (int i = 0; i < kLargerThanMaxId; i++) { ids[i].reset(nullptr); } // Creates an unique id whose max value is less than the internal