Xiao-zhen-Liu commented on PR #3962: URL: https://github.com/apache/texera/pull/3962#issuecomment-3430412233
> > Thanks for adding this temporary fix. Two comments: > > > > 1. I see we are manually doing timeout and retry in test logic. There are standard libraries, such as [Scalatest.retries](https://www.scalatest.org/scaladoc/3.0.9/org/scalatest/Retries.html). Can we use those standard library to make test more reliable instead of doing it manually? > > 2. Please follow our PR template, you are missing some questions in the template. Especially the declaration of AI part is required by ASF on changes. > > @Yicong-Huang I updated the PR description to answer all the questions. > > Regarding the retry implementation, you are right. `Scalatest.retries` is used exactly for flickery test cases that usually pass, but occasionally fail. I updated the implementation to use this instead. I tested manually by setting the timeout to be very short and the retry works. Note it only allows retry once (which should be enough for our case), and when a test case succeeds only on retry, its outcome will be `Canceled`, showing "Test Canceled: Test canceled because flickered: initially failed, but succeeded on retry". @Yicong-Huang if you agree with this new implementation we can merge the PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
