Improve consistency of Qukeys testcases

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>

tests/plugins/Qukeys/basic/shared.h

@@ -23,9 +23,9 @@
 namespace kaleidoscope {
 namespace testing {
 
-constexpr uint16_t QUKEYS_HOLD_TIMEOUT{200};
+constexpr uint16_t QUKEYS_HOLD_TIMEOUT = 200;
-constexpr uint8_t QUKEYS_OVERLAP_THRESHOLD{90};
+constexpr uint8_t QUKEYS_OVERLAP_THRESHOLD = 90;
-constexpr uint8_t QUKEYS_MINIMUM_HOLD_TIME{10};
+constexpr uint8_t QUKEYS_MINIMUM_HOLD_TIME = 10;
 
 }
 }

tests/plugins/Qukeys/basic/test/testcase.cpp

@@ -24,10 +24,6 @@ namespace kaleidoscope {
 namespace testing {
 namespace {
 
-struct QukeyValues {
-  Key primary{Key_NoKey};
-  Key alternate{Key_NoKey};
-};
 constexpr KeyAddr key_addr_A{2, 1};
 constexpr KeyAddr key_addr_S{2, 2};
 constexpr KeyAddr key_addr_D{2, 3};
@@ -36,155 +32,182 @@ constexpr KeyAddr key_addr_X{3, 2};
 
 using ::testing::IsEmpty;
 
-class QukeysBasic : public VirtualDeviceTest {};
+class QukeysBasic : public VirtualDeviceTest {
+ protected:
+  std::set<uint8_t> expected_keycodes_ = {};
+  std::unique_ptr<State> state_ = nullptr;
+};
 
 TEST_F(QukeysBasic, TapQukeyAlone) {
 
-  std::unique_ptr<State> state{nullptr};
-  std::set<uint8_t> expected_keycodes{};
-
   // Press `A`
   sim_.Press(key_addr_A);
 
-  state = VirtualDeviceTest::RunCycle();
+  state_ = RunCycle();
-
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 0);
 
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0)
+      << "There should be no HID report after the qukey is pressed";
+  sim_.RunForMillis(20);
   // Release `A`
   sim_.Release(key_addr_A);
-  expected_keycodes.insert(Key_A.getKeyCode());
-
-  state = VirtualDeviceTest::RunCycle();
-
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
-
-  expected_keycodes.erase(Key_A.getKeyCode());
 
   sim_.RunCycles(2);
-  state = VirtualDeviceTest::RunCycle();
+  state_ = RunCycle();
 
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 2)
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+      << "There should be two HID reports after the release of a tapped qukey";
-              ::testing::ElementsAreArray(expected_keycodes));
 
-  state = VirtualDeviceTest::RunCycle();
+  expected_keycodes_.insert(Key_A.getKeyCode());
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The first report should include only `A`";
 
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 0);
+  expected_keycodes_.erase(Key_A.getKeyCode());
+  EXPECT_THAT(state_->HIDReports()->Keyboard(1).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The second report should be empty";
+
+  state_ = RunCycle();
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0);
 }
 
 TEST_F(QukeysBasic, HoldQukeyAlone) {
 
-  std::unique_ptr<State> state{nullptr};
-  std::set<uint8_t> expected_keycodes{};
-
   // Press `A`
   sim_.Press(key_addr_A);
 
-  state = VirtualDeviceTest::RunCycle();
+  state_ = RunCycle();
 
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 0);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0);
 
   uint32_t t0 = Kaleidoscope.millisAtCycleStart();
 
+  // To test the hold timeout, we check after every cycle to see if there's new
+  // HID report. We can't just call `RunForMillis()` because we care about when
+  // that report was sent.
   do {
-    state = VirtualDeviceTest::RunCycle();
+    state_ = RunCycle();
-    if (Kaleidoscope.millisAtCycleStart() > t0 + QUKEYS_HOLD_TIMEOUT)
+  } while (state_->HIDReports()->Keyboard().size() == 0 &&
-      break;
+           (Kaleidoscope.millisAtCycleStart() - t0 < QUKEYS_HOLD_TIMEOUT + 1));
-  } while (state->HIDReports()->Keyboard().size() == 0);
 
   uint32_t t1 = Kaleidoscope.millisAtCycleStart();
 
-  EXPECT_THAT(t1 - t0, ::testing::Ge(QUKEYS_HOLD_TIMEOUT));
+  EXPECT_THAT(t1 - t0, ::testing::Ge(QUKEYS_HOLD_TIMEOUT))
+      << "The HID report should be sent after the hold timeout has elapsed";
 
-  expected_keycodes.insert(Key_LeftGui.getKeyCode());
+  expected_keycodes_.insert(Key_LeftGui.getKeyCode());
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1)
+      << "There should be only one HID report";
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The HID report should contain just `Key_LeftGui`";
 
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
+  sim_.RunForMillis(100);
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
 
   sim_.Release(key_addr_A);
-  expected_keycodes.erase(Key_LeftGui.getKeyCode());
+  sim_.RunCycles(2);
+  state_ = RunCycle();
 
-  sim_.RunForMillis(100);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1)
-  state = VirtualDeviceTest::RunCycle();
+      << "There should be a HID report immediately after the key is released";
 
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
+  expected_keycodes_.erase(Key_LeftGui.getKeyCode());
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The HID report should be empty at this point";
+
+  state_ = RunCycle();
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0);
 }
 
 TEST_F(QukeysBasic, FullOverlap) {
 
-  std::set<uint8_t> expected_keycodes{};
-
   sim_.Press(key_addr_F);
   sim_.RunForMillis(20);
   sim_.Press(key_addr_X);
-  auto state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 0);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0)
+      << "After both keys are pressed, there should still be no reports";
 
   sim_.RunForMillis(50);
   sim_.Release(key_addr_X);
-  state = RunCycle();
-  expected_keycodes.insert(Key_LeftShift.getKeyCode());
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
   sim_.RunCycles(3);
-  state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 2);
+
-  expected_keycodes.insert(Key_X.getKeyCode());
+  expected_keycodes_.insert(Key_LeftShift.getKeyCode());
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 3)
-              ::testing::ElementsAreArray(expected_keycodes));
+      << "After the subsequent key is released, we should get 3 reports";
-  expected_keycodes.erase(Key_X.getKeyCode());
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
-  EXPECT_THAT(state->HIDReports()->Keyboard(1).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
-              ::testing::ElementsAreArray(expected_keycodes));
+      << "The first report should contain the qukey's altername keycode";
+
+  expected_keycodes_.insert(Key_X.getKeyCode());
+  EXPECT_THAT(state_->HIDReports()->Keyboard(1).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The second report should add the subsequent key";
+
+  expected_keycodes_.erase(Key_X.getKeyCode());
+  EXPECT_THAT(state_->HIDReports()->Keyboard(2).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The third report should be the release of the subsequent key";
+
   sim_.Release(key_addr_F);
   sim_.RunCycles(3);
-  state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1)
-  expected_keycodes.erase(Key_LeftShift.getKeyCode());
+      << "After the qukey is release, we should get one report";
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+  expected_keycodes_.erase(Key_LeftShift.getKeyCode());
-              ::testing::ElementsAreArray(expected_keycodes));
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The HID report should now be empty";
+
+  state_ = RunCycle();
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0);
 }
 
 TEST_F(QukeysBasic, RolloverPrimary) {
 
-  std::set<uint8_t> expected_keycodes{};
-
   sim_.Press(key_addr_F);
   sim_.RunForMillis(20);
   sim_.Press(key_addr_X);
-  auto state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 0);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0)
+      << "After both keys are pressed, there should still be no reports";
 
   sim_.RunForMillis(50);
   sim_.Release(key_addr_F);
   sim_.RunForMillis(50);
-  state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 3);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 3)
+      << "After the qukey is released, and the overlap threshold is exceeded, there should be 3 reports";
 
-  expected_keycodes.insert(Key_F.getKeyCode());
+  expected_keycodes_.insert(Key_F.getKeyCode());
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The first report should contain the qukey's primary value";
 
-  expected_keycodes.insert(Key_X.getKeyCode());
+  expected_keycodes_.insert(Key_X.getKeyCode());
-  EXPECT_THAT(state->HIDReports()->Keyboard(1).ActiveKeycodes(),
+  EXPECT_THAT(state_->HIDReports()->Keyboard(1).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The second report should contain the subsequent (normal) key";
 
-  expected_keycodes.erase(Key_F.getKeyCode());
+  expected_keycodes_.erase(Key_F.getKeyCode());
-  EXPECT_THAT(state->HIDReports()->Keyboard(2).ActiveKeycodes(),
+  EXPECT_THAT(state_->HIDReports()->Keyboard(2).ActiveKeycodes(),
-              ::testing::ElementsAreArray(expected_keycodes));
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The third report should contain the release of the qukey";
 
   sim_.Release(key_addr_X);
   sim_.RunCycles(3);
-  state = RunCycle();
+  state_ = RunCycle();
-  ASSERT_EQ(state->HIDReports()->Keyboard().size(), 1);
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 1)
-  expected_keycodes.erase(Key_X.getKeyCode());
+      << "After the normal key is released, we should get one report";
-  EXPECT_THAT(state->HIDReports()->Keyboard(0).ActiveKeycodes(),
+  expected_keycodes_.erase(Key_X.getKeyCode());
-              ::testing::ElementsAreArray(expected_keycodes));
+  EXPECT_THAT(state_->HIDReports()->Keyboard(0).ActiveKeycodes(),
+              ::testing::ElementsAreArray(expected_keycodes_))
+      << "The HID report should now be empty";
+
+  state_ = RunCycle();
+  ASSERT_EQ(state_->HIDReports()->Keyboard().size(), 0);
 }
 
 }  // namespace