Skip to content

Conversation

fknives
Copy link
Contributor

@fknives fknives commented May 1, 2024

Overview

Close #3829

org.junit.rules.Timeout JUnit rule is not compatible with Robolectric because it starts a separate execution Thread.
Robolectric's @Test(timeout = ) uses TimeLimitedStatement which evaluates the statement on the same thread & moves the timeout & interruption into a separate thread.

Proposed Changes

Introduces a Timeout Test rule which uses the same TimeLimitedStatement to be used in place of org.junit.rules.Timeout

  • For this to work made TimeLimitedStatement instead of duplicating its functionality

@utzcoz
Copy link
Member

utzcoz commented May 2, 2024

@fknives Thanks for your contribution. Please check GitHub Action for commit message validation job and make your PR pass the CI.

@utzcoz
Copy link
Member

utzcoz commented May 2, 2024


Run # Check that the commit has a body
  # Check that the commit has a body
  commit_body="$(git log -1 --pretty=format:'%b' | grep -v 'PiperOrigin-RevId')"
  if [ -z "$commit_body" ]; then
    echo "Error: the commit should have a descriptive body"
    exit 1
  fi
  
  while read -r line; do
    if [ "${#line}" -gt 120 ] && [[ ! "$line" =~ (https?://|www\.) ]]; then
      echo "Error: the following line of the commit body is too long (max: 120 characters):"
      echo "> $line"
      exit 1
    fi
  done <<< $commit_body
  shell: /usr/bin/bash -e {0}
Error: Process completed with exit code 1.

@fknives
Copy link
Contributor Author

fknives commented May 2, 2024

@utzcoz I didn't realize, I've added a body to the commit message, sorry about that

Copy link
Member

@utzcoz utzcoz left a comment

Choose a reason for hiding this comment

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

LGTM, except some nits notes.

@utzcoz
Copy link
Member

utzcoz commented May 2, 2024

@fknives Please check Robolectric code style CI job, and ensure your code are formatted locally.

@fknives
Copy link
Contributor Author

fknives commented May 2, 2024

@utzcoz Adjusted the docs and applied google-java-format. Hopefully everything is right now 🤞 , thank you for your patience

@utzcoz
Copy link
Member

utzcoz commented May 3, 2024

Running google-java-format-diff against 'origin/master'
From https://github.com/robolectric/robolectric
 * branch                master     -> FETCH_HEAD
Please run google-java-format on the changes in this pull request
--- junit/src/main/java/org/robolectric/internal/TimeLimitedStatement.java	(before formatting)
+++ junit/src/main/java/org/robolectric/internal/TimeLimitedStatement.java	(after formatting)
@@ -5,8 +5,8 @@
 import org.junit.runners.model.TestTimedOutException;
 
 /**
- * Similar to JUnit's {@link org.junit.internal.runners.statements.FailOnTimeout}, but runs the
- * test on the current thread (with a timer on a new thread) rather than the other way around.
+ * Similar to JUnit's {@link org.junit.internal.runners.statements.FailOnTimeout}, but runs the test
+ * on the current thread (with a timer on a new thread) rather than the other way around.
  */
 public class TimeLimitedStatement extends Statement {

There still exists some format issues. You can check https://github.com/robolectric/robolectric/wiki/Robolectric's-code-style to fix it.

@fknives
Copy link
Contributor Author

fknives commented May 3, 2024

@utzcoz Right, the TimeLimitedStatement wasn't formatter properly either, ohh my

@utzcoz
Copy link
Member

utzcoz commented May 3, 2024

@fknives Please rebase your PR based on latest master branch as I merged some commits to make CI green.

@utzcoz utzcoz changed the title Create TimeoutRule which can be used with Robolectric in place of org.junit.rules.Timeout issue#3829 Create TimeoutRule which can be used with Robolectric in place of org.junit.rules.Timeout May 3, 2024
org.junit.rules.Timeout is incompatible with Robolectric's scheduler because it spawns a new thread for the test.
This commit introduces TimeoutRule which behaves like `@Test(timeout = )` but for all tests in the file
@utzcoz utzcoz requested a review from hoisie May 3, 2024 12:27
@utzcoz utzcoz merged commit 3ab3afe into robolectric:master May 3, 2024
@utzcoz
Copy link
Member

utzcoz commented May 3, 2024

@fknives Thanks for your contribution, and congrats to your first contribution for Robolectric. I think it will be released with Robolectric 4.13, and you can use Robolectric 4.13-snapshot now to test it.

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

Successfully merging this pull request may close these issues.

Running tests on other thread does not properly switch Main Looper to new thread
2 participants