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

Use a stable creation date for new revisions #422

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -37,8 +37,10 @@ private ArchiveItem(ArchiveItem parent, String id, ZonedDateTime created, ZonedD
this.footer = footer;
}

static ArchiveItem from(PullRequest pr, Repository localRepo, URI issueTracker, String issuePrefix, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, Hash base, Hash head) {
return new ArchiveItem(null, "fc", pr.createdAt(), pr.updatedAt(), pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
static ArchiveItem from(PullRequest pr, Repository localRepo, URI issueTracker, String issuePrefix,
WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification,
ZonedDateTime created, ZonedDateTime updated, Hash base, Hash head) {
return new ArchiveItem(null, "fc", created, updated, pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
() -> "RFR: " + pr.title(),
() -> "",
() -> ArchiveMessages.composeConversation(pr, base, head),
@@ -49,8 +51,10 @@ static ArchiveItem from(PullRequest pr, Repository localRepo, URI issueTracker,
});
}

static ArchiveItem from(PullRequest pr, Repository localRepo, WebrevStorage.WebrevGenerator webrevGenerator, WebrevNotification webrevNotification, Hash lastBase, Hash lastHead, Hash base, Hash head, int index, ArchiveItem parent) {
return new ArchiveItem(parent,"ha" + head.hex(), ZonedDateTime.now(), ZonedDateTime.now(), pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
static ArchiveItem from(PullRequest pr, Repository localRepo, WebrevStorage.WebrevGenerator webrevGenerator,
WebrevNotification webrevNotification, ZonedDateTime created, ZonedDateTime updated,
Hash lastBase, Hash lastHead, Hash base, Hash head, int index, ArchiveItem parent) {
return new ArchiveItem(parent,"ha" + head.hex(), created, updated, pr.author(), Map.of("PR-Head-Hash", head.hex(), "PR-Base-Hash", base.hex()),
() -> String.format("Re: [Rev %02d] RFR: %s", index, pr.title()),
() -> "",
() -> ArchiveMessages.composeRevision(pr, localRepo, base, head, lastBase, lastHead),
@@ -66,12 +66,13 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
if (email.hasHeader("PR-Base-Hash")) {
var curBase = new Hash(email.headerValue("PR-Base-Hash"));
var curHead = new Hash(email.headerValue("PR-Head-Hash"));
var created = email.date();

if (generated.isEmpty()) {
var first = ArchiveItem.from(pr, localRepo, issueTracker, issuePrefix, webrevGenerator, webrevNotification, curBase, curHead);
var first = ArchiveItem.from(pr, localRepo, issueTracker, issuePrefix, webrevGenerator, webrevNotification, pr.createdAt(), pr.updatedAt(), curBase, curHead);
generated.add(first);
} else {
var revision = ArchiveItem.from(pr, localRepo, webrevGenerator, webrevNotification, lastBase, lastHead, curBase, curHead, ++revisionIndex, generated.get(0));
var revision = ArchiveItem.from(pr, localRepo, webrevGenerator, webrevNotification, created, created, lastBase, lastHead, curBase, curHead, ++revisionIndex, generated.get(0));
generated.add(revision);
}

@@ -83,10 +84,10 @@ private List<ArchiveItem> generateArchiveItems(List<Email> sentEmails, Repositor
// Check if we're at a revision not previously reported
if (!base.equals(lastBase) || !head.equals(lastHead)) {
if (generated.isEmpty()) {
var first = ArchiveItem.from(pr, localRepo, issueTracker, issuePrefix, webrevGenerator, webrevNotification, base, head);
var first = ArchiveItem.from(pr, localRepo, issueTracker, issuePrefix, webrevGenerator, webrevNotification, pr.createdAt(), pr.updatedAt(), base, head);
generated.add(first);
} else {
var revision = ArchiveItem.from(pr, localRepo, webrevGenerator, webrevNotification, lastBase, lastHead, base, head, ++revisionIndex, generated.get(0));
var revision = ArchiveItem.from(pr, localRepo, webrevGenerator, webrevNotification, pr.updatedAt(), pr.updatedAt(), lastBase, lastHead, base, head, ++revisionIndex, generated.get(0));
generated.add(revision);
}
}
@@ -1526,6 +1526,72 @@ void cooldown(TestInfo testInfo) throws IOException {
}
}

@Test
void cooldownNewRevision(TestInfo testInfo) throws IOException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory();
var archiveFolder = new TemporaryDirectory();
var listServer = new TestMailmanServer();
var webrevServer = new TestWebrevServer()) {
var bot = credentials.getHostedRepository();
var author = credentials.getHostedRepository();
var archive = credentials.getHostedRepository();
var listAddress = EmailAddress.parse(listServer.createList("test"));
var censusBuilder = credentials.getCensusBuilder()
.addAuthor(author.forge().currentUser().id());
var from = EmailAddress.from("test", "test@test.mail");
var mlBotBuilder = MailingListBridgeBot.newBuilder()
.from(from)
.repo(bot)
.ignoredUsers(Set.of(bot.forge().currentUser().userName()))
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
.webrevStorageRepository(archive)
.webrevStorageRef("webrev")
.webrevStorageBase(Path.of("test"))
.webrevStorageBaseUri(webrevServer.uri())
.issueTracker(URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile);
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);
localRepo.push(masterHash, archive.url(), "webrev", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo, "Line 1\nLine 2\nLine 3\nLine 4");
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request");
pr.setBody("This is now ready");

var mlBot = mlBotBuilder.build();
var mlBotWithCooldown = mlBotBuilder.cooldown(Duration.ofDays(1)).build();

TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// Commit another revision
var updatedHash = CheckableRepository.appendAndCommit(localRepo, "More stuff");
localRepo.push(updatedHash, author.url(), "edit");

// Bot with cooldown configured should not create a new webrev
TestBotRunner.runPeriodicItems(mlBotWithCooldown);
assertThrows(RuntimeException.class, () -> listServer.processIncoming(Duration.ofMillis(1)));

// But without, it should
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// Check the archive
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "webrev.01"));
}
}

@Test
void retryAfterCooldown(TestInfo testInfo) throws IOException, InterruptedException {
try (var credentials = new HostCredentials(testInfo);
@@ -1604,4 +1670,84 @@ void retryAfterCooldown(TestInfo testInfo) throws IOException, InterruptedExcept
assertTrue(archiveContains(archiveFolder.path(), "Looks good - " + counter + " -"));
}
}

@Test
void retryNewRevisionAfterCooldown(TestInfo testInfo) throws IOException, InterruptedException {
try (var credentials = new HostCredentials(testInfo);
var tempFolder = new TemporaryDirectory();
var archiveFolder = new TemporaryDirectory();
var listServer = new TestMailmanServer();
var webrevServer = new TestWebrevServer()) {
var bot = credentials.getHostedRepository();
var author = credentials.getHostedRepository();
var archive = credentials.getHostedRepository();
var listAddress = EmailAddress.parse(listServer.createList("test"));
var censusBuilder = credentials.getCensusBuilder()
.addAuthor(author.forge().currentUser().id());
var from = EmailAddress.from("test", "test@test.mail");
var cooldown = Duration.ofMillis(500);
var mlBotBuilder = MailingListBridgeBot.newBuilder()
.from(from)
.repo(bot)
.ignoredUsers(Set.of(bot.forge().currentUser().userName()))
.archive(archive)
.censusRepo(censusBuilder.build())
.list(listAddress)
.listArchive(listServer.getArchive())
.smtpServer(listServer.getSMTP())
.webrevStorageRepository(archive)
.webrevStorageRef("webrev")
.webrevStorageBase(Path.of("test"))
.webrevStorageBaseUri(webrevServer.uri())
.issueTracker(URIBuilder.base("http://issues.test/browse/").build());

// Populate the projects repository
var reviewFile = Path.of("reviewfile.txt");
var localRepo = CheckableRepository.init(tempFolder.path(), author.repositoryType(), reviewFile);
var masterHash = localRepo.resolve("master").orElseThrow();
localRepo.push(masterHash, author.url(), "master", true);
localRepo.push(masterHash, archive.url(), "webrev", true);

// Make a change with a corresponding PR
var editHash = CheckableRepository.appendAndCommit(localRepo, "Line 1\nLine 2\nLine 3\nLine 4");
localRepo.push(editHash, author.url(), "edit", true);
var pr = credentials.createPullRequest(archive, "master", "edit", "This is a pull request");
pr.setBody("This is now ready");

var mlBot = mlBotBuilder.cooldown(cooldown).build();
Thread.sleep(cooldown.toMillis());
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// Make a new revision and run the check within the cooldown period
int counter;
for (counter = 1; counter < 10; ++counter) {
var start = Instant.now();
var revHash = CheckableRepository.appendAndCommit(localRepo, "more stuff", "Update number - " + counter + " -");
localRepo.push(revHash, author.url(), "edit");

// The bot should not bridge the new revision due to cooldown
TestBotRunner.runPeriodicItems(mlBot);
var elapsed = Duration.between(start, Instant.now());
if (elapsed.compareTo(cooldown) < 0) {
break;
} else {
log.info("Didn't do the test in time - retrying (elapsed: " + elapsed + " required: " + cooldown + ")");
listServer.processIncoming();
cooldown = cooldown.multipliedBy(2);
mlBot = mlBotBuilder.cooldown(cooldown).build();
}
}
assertThrows(RuntimeException.class, () -> listServer.processIncoming(Duration.ofMillis(1)));

// But after the cooldown period has passed, it should
Thread.sleep(cooldown.toMillis());
TestBotRunner.runPeriodicItems(mlBot);
listServer.processIncoming();

// Check the archive
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "Update number - " + counter + " -"));
}
}
}