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

mlbridge: drop prefix "webrev" from webrev paths #718

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
@@ -222,7 +222,7 @@ private URI createAndArchive(PullRequest pr, Repository localRepository, Path sc
try {
var localStorage = Repository.materialize(scratchPath, storage.url(),
"+" + storageRef + ":mlbridge_webrevs");
var relativeFolder = baseFolder.resolve(String.format("%s/webrev.%s", pr.id(), identifier));
var relativeFolder = baseFolder.resolve(String.format("%s/%s", pr.id(), identifier));
var outputFolder = scratchPath.resolve(relativeFolder);
// If a previous operation was interrupted there may be content here already - overwrite if so
if (Files.exists(outputFolder)) {
@@ -194,7 +194,7 @@ void simpleArchive(TestInfo testInfo) throws IOException {
assertTrue(archiveContains(archiveFolder.path(), "Changes:"));
assertTrue(archiveContains(archiveFolder.path(), "Webrev:"));
assertTrue(archiveContains(archiveFolder.path(), webrevServer.uri().toString()));
assertTrue(archiveContains(archiveFolder.path(), "webrev.00"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/00"));
assertTrue(archiveContains(archiveFolder.path(), "Issue:"));
assertTrue(archiveContains(archiveFolder.path(), "http://issues.test/browse/TSTPRJ-1234"));
assertTrue(archiveContains(archiveFolder.path(), "Fetch:"));
@@ -1553,8 +1553,8 @@ void incrementalChanges(TestInfo testInfo) throws IOException {
// The archive should reference the updated push
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "has updated the pull request incrementally"));
assertTrue(archiveContains(archiveFolder.path(), "full.*/" + pr.id() + "/webrev.01"));
assertTrue(archiveContains(archiveFolder.path(), "inc.*/" + pr.id() + "/webrev.00-01"));
assertTrue(archiveContains(archiveFolder.path(), "full.*/" + pr.id() + "/01"));
assertTrue(archiveContains(archiveFolder.path(), "inc.*/" + pr.id() + "/00-01"));
assertTrue(archiveContains(archiveFolder.path(), "Patch"));
assertTrue(archiveContains(archiveFolder.path(), "Fetch"));
assertTrue(archiveContains(archiveFolder.path(), "Fixing"));
@@ -1682,7 +1682,7 @@ void rebased(TestInfo testInfo) throws IOException {
// The archive should reference the rebased push
Repository.materialize(archiveFolder.path(), archive.url(), "master");
assertTrue(archiveContains(archiveFolder.path(), "has updated the pull request with a new target base"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/webrev.01"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/01"));
assertFalse(archiveContains(archiveFolder.path(), "Incremental"));
assertTrue(archiveContains(archiveFolder.path(), "Patch"));
assertTrue(archiveContains(archiveFolder.path(), "Fetch"));
@@ -1794,8 +1794,8 @@ void incrementalAfterRebase(TestInfo testInfo) throws IOException {
Repository.materialize(archiveFolder.path(), archive.url(), "archive");
assertTrue(archiveContains(archiveFolder.path(), "has updated the pull request with a new target base"));
assertTrue(archiveContains(archiveFolder.path(), "excludes"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/webrev.01"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/webrev.00-01"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/01"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/00-01"));
assertTrue(archiveContains(archiveFolder.path(), "Original msg"));
assertTrue(archiveContains(archiveFolder.path(), "More updates"));
}
@@ -1876,7 +1876,7 @@ void mergeWebrev(TestInfo testInfo) throws IOException {
// The archive should contain a merge style webrev
Repository.materialize(archiveFolder.path(), archive.url(), "archive");
assertTrue(archiveContains(archiveFolder.path(), "The webrevs contain the adjustments done while merging with regards to each parent branch:"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/webrev.00.0"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/00.0"));
assertTrue(archiveContains(archiveFolder.path(), "3 lines in 2 files changed: 1 ins; 1 del; 1 mod"));

// The PR should contain a webrev comment
@@ -1949,7 +1949,7 @@ void mergeWebrevConflict(TestInfo testInfo) throws IOException {
// The archive should contain a merge style webrev
Repository.materialize(archiveFolder.path(), archive.url(), "archive");
assertTrue(archiveContains(archiveFolder.path(), "The webrev contains the conflicts with master:"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/webrev.00.conflicts"));
assertTrue(archiveContains(archiveFolder.path(), pr.id() + "/00.conflicts"));
assertTrue(archiveContains(archiveFolder.path(), "2 lines in 2 files changed: 2 ins; 0 del; 0 mod"));

// The PR should contain a webrev comment
@@ -2090,7 +2090,7 @@ void skipAddingExistingWebrev(TestInfo testInfo) throws IOException {
.filter(comment -> comment.body().contains(editHash.hex()))
.collect(Collectors.toList());
assertEquals(1, webrevComments.size());
assertEquals(1, countSubstrings(webrevComments.get(0).body(), "webrev.00"));
assertEquals(1, countSubstrings(webrevComments.get(0).body(), pr.id() + "/00"));

// Pretend the archive didn't work out
archiveRepo.push(masterHash, archive.url(), "master", true);
@@ -2106,7 +2106,7 @@ void skipAddingExistingWebrev(TestInfo testInfo) throws IOException {
.filter(comment -> comment.body().contains(editHash.hex()))
.collect(Collectors.toList());
assertEquals(1, webrevComments.size());
assertEquals(1, countSubstrings(webrevComments.get(0).body(), "webrev.00"));
assertEquals(1, countSubstrings(webrevComments.get(0).body(), pr.id() + "/00"));
}
}

@@ -2454,7 +2454,7 @@ void cooldownNewRevision(TestInfo testInfo) throws IOException {

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

@@ -76,12 +76,12 @@ void overwriteExisting(TestInfo testInfo) throws IOException {

// Update the local repository and check that the webrev has been generated
Repository.materialize(repoFolder, archive.url(), "webrev");
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/webrev.00/index.html")));
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/00/index.html")));

// Create it again - it will overwrite the previous one
generator.generate(masterHash, editHash, "00", WebrevDescription.Type.FULL);
Repository.materialize(repoFolder, archive.url(), "webrev");
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/webrev.00/index.html")));
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/00/index.html")));
}
}

@@ -125,9 +125,9 @@ void dropLarge(TestInfo testInfo) throws IOException {

// Update the local repository and check that the webrev has been generated
Repository.materialize(repoFolder, archive.url(), "webrev");
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/webrev.00/index.html")));
assertTrue(Files.size(repoFolder.resolve("test/" + pr.id() + "/webrev.00/large.txt")) > 0);
assertTrue(Files.size(repoFolder.resolve("test/" + pr.id() + "/webrev.00/large.txt")) < 1000);
assertTrue(Files.exists(repoFolder.resolve("test/" + pr.id() + "/00/index.html")));
assertTrue(Files.size(repoFolder.resolve("test/" + pr.id() + "/00/large.txt")) > 0);
assertTrue(Files.size(repoFolder.resolve("test/" + pr.id() + "/00/large.txt")) < 1000);
}
}
}