Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new test cases for Timer autostart and paused behavior #97794

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arangain
Copy link

@arangain arangain commented Oct 4, 2024

This pull request adds new unit tests for the Timer class to help improve test coverage as part of issue #43440. The tests cover the following scenarios:

  • Autostart behavior: Ensures that when the autostart property is enabled, the timer starts automatically when added to the scene.
  • Paused timer behavior: Confirms that when a timer is paused, it doesn't process time and that the remaining time is preserved correctly until the timer is resumed.

Link to pull request issue: #43440

Thanks!

@arangain arangain requested a review from a team as a code owner October 4, 2024 00:31
Copy link
Contributor

@RadiantUwU RadiantUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, we should run the workflow and make sure the tests pass.

@AThousandShips AThousandShips changed the title Added new test cases for Timer autostart and paused behavior (#43440) Add new test cases for Timer autostart and paused behavior Oct 4, 2024
@AThousandShips AThousandShips added this to the 4.x milestone Oct 4, 2024
@Mickeon
Copy link
Contributor

Mickeon commented Oct 4, 2024

It did not, in fact, pass. Most of the errors are about replacing the spaces with tabulation.

Copy link
Contributor

@Mickeon Mickeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe most of these comments are necessary. The code should speak for itself. If comments are necessary, it seems like the SUBCASE macro in this same file is more explicit than a comment.

@@ -212,6 +212,46 @@ TEST_CASE("[SceneTree][Timer] Check Timer timeout signal") {
memdelete(test_timer);
}

// Test case for timer autostart feature.
// Ensures that when autostart is enabled, the timer starts automatically when added to scene.
TEST_CASE("[SceneTree][Timer] Autostart behavior") {
Copy link
Contributor

@Mickeon Mickeon Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent the review too early but

Suggested change
TEST_CASE("[SceneTree][Timer] Autostart behavior") {
TEST_CASE("[SceneTree][Timer] Check Timer Autostart behavior") {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants