Use reflection to inject the attempt number

This CL is to address the public static field in RepeatableRunner
for caller to get the current attempt number. We tried to have
a JUnit TestRule to achieve the purpose but it ended up with having
a RuleChain in each class where we already have multiple rules and
need to add the retry rule. This is because we have to make sure
the retry rule is the last one to wrap the test statement so that
the actual retry can include the actions defined in other rules.
Having a rule chain is not scalable and confuses engineer so we
gave it up.

Instead, we decided to expand the current RepeatableRunner to
use reflection to inject the attempt number to the test class.
Doing it this way can reduce the burden from the caller and it also
gets rid of the global state from the previous public static field.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=240789045
This commit is contained in:
shicong 2019-03-28 09:34:08 -07:00 committed by jianglai
parent 174d8d69ea
commit 8af9090fb0
9 changed files with 132 additions and 45 deletions

View file

@ -16,7 +16,6 @@ package google.registry.webdriver;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static google.registry.webdriver.RepeatableRunner.currentAttemptIndex;
import static java.lang.Math.abs;
import static java.util.stream.Collectors.joining;
@ -123,9 +122,9 @@ class ChromeWebDriverPlusScreenDiffer implements WebDriverPlusScreenDiffer {
}
@Override
public void diffElement(String imageKey, WebElement element) {
public void diffElement(WebElement element, String imageKey, int attempt) {
ActualScreenshot elementImage =
takeScreenshot(imageKey)
ActualScreenshot.create(imageKey, attempt, takeScreenshot())
.getSubimage(
element.getLocation().getX(),
element.getLocation().getY(),
@ -135,8 +134,8 @@ class ChromeWebDriverPlusScreenDiffer implements WebDriverPlusScreenDiffer {
}
@Override
public void diffPage(String imageKey) {
actualScreenshots.add(takeScreenshot(imageKey));
public void diffPage(String imageKey, int attempt) {
actualScreenshots.add(ActualScreenshot.create(imageKey, attempt, takeScreenshot()));
}
@Override
@ -190,10 +189,10 @@ class ChromeWebDriverPlusScreenDiffer implements WebDriverPlusScreenDiffer {
}
}
private ActualScreenshot takeScreenshot(String imageKey) {
private byte[] takeScreenshot() {
checkArgument(webDriver instanceof TakesScreenshot);
TakesScreenshot takesScreenshot = (TakesScreenshot) webDriver;
return ActualScreenshot.create(imageKey, takesScreenshot.getScreenshotAs(OutputType.BYTES));
return takesScreenshot.getScreenshotAs(OutputType.BYTES);
}
private ComparisonResult compareScreenshots(ActualScreenshot screenshot) {
@ -246,7 +245,7 @@ class ChromeWebDriverPlusScreenDiffer implements WebDriverPlusScreenDiffer {
File thisScreenshotDir =
Paths.get(
screenshotDir,
"attempt_" + currentAttemptIndex,
"attempt_" + result.actualScreenshot().getAttempt(),
result.isConsideredSimilar() ? "similar" : "different")
.toFile();
thisScreenshotDir.mkdirs();