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

Retry test with delay #2277

Open
yadavan88 opened this issue Sep 18, 2023 · 5 comments
Open

Retry test with delay #2277

yadavan88 opened this issue Sep 18, 2023 · 5 comments

Comments

@yadavan88
Copy link

Hi,
I am trying to retry a failing test multiple times. I want to set a delay of a few seconds before the next retry. I thought I could use the withRetry method by passing the delay time as Span. However, it doesn't seem to work as I expected.

Here is a sample code:

@Retryable
class RetrySpec extends AnyWordSpec with Matchers with Retries {
  val retries = 5
  override def withFixture(test: NoArgTest): Outcome = {
    if (isRetryable(test)) withFixture(test, retries)
    else super.withFixture(test)
  }

  def withFixture(test: NoArgTest, count: Int): Outcome = {
    val outcome = super.withFixture(test)
    outcome match {
      case Failed(_) | Canceled(_) =>
        if (count == 1) super.withFixture(test)
        else {
          println(
            s"Retrying flaky test  `${test.name}`, Attempts remaining: ${count - 1}"
          )
          withRetry(Span(2, Seconds))(withFixture(test, count - 1))
        }
      case other => other
    }
  }

  "the test" should {
    "retry" in {
      println("inside the test >>>>>>>> " + LocalDateTime.now())
      fail("Failing test.... ")
    }

  }
}

I wanted to retry the same test but after a delay of 2 seconds. Is this not the correct way?

@cheeseng
Copy link
Contributor

@yadavan88 I think you discovered a bug, the source of the withRetry that you are calling:

https://github.com/scalatest/scalatest/blob/3.2.x-new/jvm/core/src/main/scala/org/scalatest/Retries.scala#L344

As you see, there's no use of the passed in delay to sleep to wait. On thing to note though, withRetry will call the passed in code 2 times when it fails the first time, so it may not be the one that you desired, perhaps something like the following is nearer to what you are after:

import org.scalatest._
import org.scalatest.wordspec.AnyWordSpec
import org.scalatest.matchers.should.Matchers
import org.scalatest.tags.Retryable
import java.time.LocalDateTime

@Retryable
class TestRetrySpec extends AnyWordSpec with Matchers with Retries {
  val retries = 5
  override def withFixture(test: NoArgTest): Outcome = {
    if (isRetryable(test)) withFixture(test, retries)
    else super.withFixture(test)
  }

  def withFixture(test: NoArgTest, count: Int): Outcome = {
    val outcome = super.withFixture(test)
    outcome match {
      case Failed(_) | Canceled(_) =>
        if (count == 1) super.withFixture(test)
        else {
          println(
            s"Retrying flaky test  `${test.name}`, Attempts remaining: ${count - 1}"
          )
          Thread.sleep(2000)
          withFixture(test, count - 1)
        }
      case other => other
    }
  }

  "the test" should {
    "retry" in {
      println("inside the test >>>>>>>> " + LocalDateTime.now())
      fail("Failing test.... ")
    }

  }
}

I'll submit a PR to fix the bug you discovered, at the same time hopefully the above example can help you move forward.

Cheers.

@yadavan88
Copy link
Author

Thanks @cheeseng for the info and help. :)

Just one more question, does it make sense to provide the retry ability out of the box for the default case?
For example, by annotation or a tag that uses a number of attempts like in utest?

@cheeseng
Copy link
Contributor

@yadavan88 I think it is a good idea to have overloaded withRetry/withRetries functions that takes a maximum count, that shouldn't be hard, I'll try working out a PR targeting for the upcoming 3.3 and see if @bvenners thinks it is a good idea.

@yadavan88
Copy link
Author

Thank you @cheeseng :)

@cheeseng
Copy link
Contributor

@yadavan88 Fyi I submitted the PR to add withRetries here:

#2282

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

No branches or pull requests

2 participants