From 637f254ecb00931f87fc085f4bb001b0d0f8b0e5 Mon Sep 17 00:00:00 2001 From: Kaya <95276965+kytpbs@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:40:30 +0300 Subject: [PATCH] move some tests to `schedulingRecursionTest` & improve comments --- .../wpilibj2/command/CommandScheduleTest.java | 44 ------------------- .../command/SchedulingRecursionTest.java | 43 ++++++++++++++++++ .../cpp/frc2/command/CommandScheduleTest.cpp | 41 +---------------- .../frc2/command/SchedulingRecursionTest.cpp | 39 ++++++++++++++++ 4 files changed, 83 insertions(+), 84 deletions(-) diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandScheduleTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandScheduleTest.java index 117d694e62b..d33cfdc3b9f 100644 --- a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandScheduleTest.java +++ b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/CommandScheduleTest.java @@ -111,50 +111,6 @@ void schedulerCancelTest() { } } - @Test - void cancelNextCommandTest() { - try (CommandScheduler scheduler = new CommandScheduler()) { - Command[] commands = new Command[2]; - var commandRunCounter = new AtomicInteger(0); - // for parity with C++ where the ordering of sets is non-deterministic, we cancel the other - // command in the - // first command and check that the second command is not run - Command command1 = - new RunCommand( - () -> { - scheduler.cancel(commands[1]); - commandRunCounter.incrementAndGet(); - }); - Command command2 = - new RunCommand( - () -> { - scheduler.cancel(commands[0]); - commandRunCounter.incrementAndGet(); - }); - - commands[0] = command1; - commands[1] = command2; - - scheduler.schedule(command1, command2); - scheduler.run(); - - assertEquals( - 1, commandRunCounter.get(), "Second command was run when it shouldn't have been"); - - // only one of the commands should be canceled. - assertFalse( - scheduler.isScheduled(command1) && scheduler.isScheduled(command2), - "None of the commands were canceled when one should have been"); - // one of the commands shouldn't be canceled because the other one is canceled first - assertTrue( - scheduler.isScheduled(command1) || scheduler.isScheduled(command2), - "Both commands were canceled when only one should have been"); - - scheduler.run(); - assertEquals(2, commandRunCounter.get()); - } - } - @Test void commandKnowsWhenEndedTest() { try (CommandScheduler scheduler = new CommandScheduler()) { diff --git a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java index a253740c028..f5a6afa4799 100644 --- a/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java +++ b/wpilibNewCommands/src/test/java/edu/wpi/first/wpilibj2/command/SchedulingRecursionTest.java @@ -462,4 +462,47 @@ void cancelDefaultCommandFromEnd() { assertTrue(scheduler.isScheduled(other)); } } + + @Test + void cancelNextCommandFromCommandTest() { + try (CommandScheduler scheduler = new CommandScheduler()) { + Command[] commands = new Command[2]; + var commandRunCounter = new AtomicInteger(0); + // for parity with C++ where the ordering of sets is non-deterministic, we cancel the other + // command in the first command and check that the second command is not run + Command command1 = + new RunCommand( + () -> { + scheduler.cancel(commands[1]); + commandRunCounter.incrementAndGet(); + }); + Command command2 = + new RunCommand( + () -> { + scheduler.cancel(commands[0]); + commandRunCounter.incrementAndGet(); + }); + + commands[0] = command1; + commands[1] = command2; + + scheduler.schedule(command1, command2); + scheduler.run(); + + assertEquals( + 1, commandRunCounter.get(), "Second command was run when it shouldn't have been"); + + // only one of the commands should be canceled. + assertFalse( + scheduler.isScheduled(command1) && scheduler.isScheduled(command2), + "None of the commands were canceled when one should have been"); + // one of the commands shouldn't be canceled because the other one is canceled first + assertTrue( + scheduler.isScheduled(command1) || scheduler.isScheduled(command2), + "Both commands were canceled when only one should have been"); + + scheduler.run(); + assertEquals(2, commandRunCounter.get()); + } + } } diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandScheduleTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandScheduleTest.cpp index b7d46869530..675e21342b6 100644 --- a/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandScheduleTest.cpp +++ b/wpilibNewCommands/src/test/native/cpp/frc2/command/CommandScheduleTest.cpp @@ -101,45 +101,6 @@ TEST_F(CommandScheduleTest, SchedulerCancel) { EXPECT_FALSE(scheduler.IsScheduled(&command)); } -TEST_F(CommandScheduleTest, CancelNextCommand) { - CommandScheduler scheduler = GetScheduler(); - - frc2::RunCommand* command1Ptr = nullptr; - frc2::RunCommand* command2Ptr = nullptr; - int counter = 0; - - auto command1 = frc2::RunCommand([&counter, &command2Ptr, &scheduler] { - scheduler.Cancel(command2Ptr); - counter++; - }); - auto command2 = frc2::RunCommand([&counter, &command1Ptr, &scheduler] { - scheduler.Cancel(command1Ptr); - counter++; - }); - - command1Ptr = &command1; - command2Ptr = &command2; - - scheduler.Schedule(&command1); - scheduler.Schedule(&command2); - scheduler.Run(); - - EXPECT_EQ(counter, 1) << "Second command was run when it shouldn't have been"; - - // only one of the commands should be canceled. - EXPECT_FALSE(scheduler.IsScheduled(&command1) && - scheduler.IsScheduled(&command2)) - << "Both commands are running when only one should be"; - // one of the commands shouldn't be canceled because the other one is canceled - // first - EXPECT_TRUE(scheduler.IsScheduled(&command1) || - scheduler.IsScheduled(&command2)) - << "Both commands are canceled when only one should be"; - - scheduler.Run(); - EXPECT_EQ(counter, 2); -} - TEST_F(CommandScheduleTest, CommandKnowsWhenItEnded) { CommandScheduler scheduler = GetScheduler(); @@ -169,7 +130,7 @@ TEST_F(CommandScheduleTest, ScheduleCommandInCommand) { frc2::RunCommand([&counter, &scheduler, &commandToGetScheduled] { scheduler.Schedule(&commandToGetScheduled); EXPECT_EQ(counter, 1) - << "Scheduled cmmand's init was not run immediately " + << "Scheduled command's init was not run immediately " "after getting scheduled"; }); diff --git a/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp b/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp index 52436b97ac6..0b54c589515 100644 --- a/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp +++ b/wpilibNewCommands/src/test/native/cpp/frc2/command/SchedulingRecursionTest.cpp @@ -339,6 +339,45 @@ TEST_F(SchedulingRecursionTest, CancelDefaultCommandFromEnd) { EXPECT_TRUE(scheduler.IsScheduled(&other)); } +TEST_F(SchedulingRecursionTest, CancelNextCommandFromCommand) { + CommandScheduler scheduler = GetScheduler(); + + frc2::RunCommand* command1Ptr = nullptr; + frc2::RunCommand* command2Ptr = nullptr; + int counter = 0; + + auto command1 = frc2::RunCommand([&counter, &command2Ptr, &scheduler] { + scheduler.Cancel(command2Ptr); + counter++; + }); + auto command2 = frc2::RunCommand([&counter, &command1Ptr, &scheduler] { + scheduler.Cancel(command1Ptr); + counter++; + }); + + command1Ptr = &command1; + command2Ptr = &command2; + + scheduler.Schedule(&command1); + scheduler.Schedule(&command2); + scheduler.Run(); + + EXPECT_EQ(counter, 1) << "Second command was run when it shouldn't have been"; + + // only one of the commands should be canceled. + EXPECT_FALSE(scheduler.IsScheduled(&command1) && + scheduler.IsScheduled(&command2)) + << "Both commands are running when only one should be"; + // one of the commands shouldn't be canceled because the other one is canceled + // first + EXPECT_TRUE(scheduler.IsScheduled(&command1) || + scheduler.IsScheduled(&command2)) + << "Both commands are canceled when only one should be"; + + scheduler.Run(); + EXPECT_EQ(counter, 2); +} + INSTANTIATE_TEST_SUITE_P( SchedulingRecursionTests, SchedulingRecursionTest, testing::Values(Command::InterruptionBehavior::kCancelSelf,