Skip to content

Commit

Permalink
8343610: InOutPathTest jpackage test produces invalid app image on macOS
Browse files Browse the repository at this point in the history
Reviewed-by: almatvee
  • Loading branch information
Alexey Semenyuk committed Nov 7, 2024
1 parent f0b251d commit ac82a8f
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -897,8 +897,9 @@ public JPackageCommand setAppLayoutAsserts(AppLayoutAssert ... asserts) {
}

public JPackageCommand excludeAppLayoutAsserts(AppLayoutAssert... asserts) {
return setAppLayoutAsserts(Stream.of(asserts).filter(Predicate.not(
appLayoutAsserts::contains)).toArray(AppLayoutAssert[]::new));
var asSet = Set.of(asserts);
return setAppLayoutAsserts(appLayoutAsserts.stream().filter(Predicate.not(
asSet::contains)).toArray(AppLayoutAssert[]::new));
}

JPackageCommand assertAppLayout() {
Expand Down
24 changes: 21 additions & 3 deletions test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ static PackageHandlers createPkgPackageHandlers() {
}

static void verifyBundleStructure(JPackageCommand cmd) {
Path bundleRoot;
final Path bundleRoot;
if (cmd.isImagePackageType()) {
bundleRoot = cmd.outputBundle();
} else {
Expand All @@ -268,8 +268,26 @@ static void verifyBundleStructure(JPackageCommand cmd) {
}

TKit.assertDirectoryContent(bundleRoot).match(Path.of("Contents"));
TKit.assertDirectoryContent(bundleRoot.resolve("Contents")).match(
cmd.isRuntime() ? RUNTIME_BUNDLE_CONTENTS : APP_BUNDLE_CONTENTS);

final var contentsDir = bundleRoot.resolve("Contents");
final var expectedContentsItems = cmd.isRuntime() ? RUNTIME_BUNDLE_CONTENTS : APP_BUNDLE_CONTENTS;

var contentsVerifier = TKit.assertDirectoryContent(contentsDir);
if (!cmd.hasArgument("--app-content")) {
contentsVerifier.match(expectedContentsItems);
} else {
// Additional content added to the bundle.
// Verify there is no period (.) char in the names of additional directories if any.
contentsVerifier.contains(expectedContentsItems);
contentsVerifier = contentsVerifier.removeAll(expectedContentsItems);
contentsVerifier.match(contentsVerifier.items().stream().filter(path -> {
if (Files.isDirectory(contentsDir.resolve(path))) {
return !path.getFileName().toString().contains(".");
} else {
return true;
}
}).collect(toSet()));
}
}

static String getBundleName(JPackageCommand cmd) {
Expand Down
14 changes: 11 additions & 3 deletions test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ public void match(Set<Path> expected) {
baseDir, format(comm.common), format(comm.unique1), format(comm.unique2)));
} else if (!comm.unique1.isEmpty()) {
error(String.format(
"assertDirectoryContentEquals%s: Expected %s. Unexpected %s",
"assertDirectoryContentEquals(%s): Expected %s. Unexpected %s",
baseDir, format(comm.common), format(comm.unique1)));
} else if (!comm.unique2.isEmpty()) {
error(String.format(
Expand Down Expand Up @@ -818,12 +818,20 @@ public void contains(Set<Path> expected) {
}
}

public DirectoryContentVerifier removeAll(Path ... paths) {
public DirectoryContentVerifier removeAll(Collection<Path> paths) {
Set<Path> newContent = new HashSet<>(content);
newContent.removeAll(List.of(paths));
newContent.removeAll(paths);
return new DirectoryContentVerifier(baseDir, newContent);
}

public DirectoryContentVerifier removeAll(Path ... paths) {
return removeAll(List.of(paths));
}

public Set<Path> items() {
return content;
}

private DirectoryContentVerifier(Path baseDir, Set<Path> contents) {
this.baseDir = baseDir;
this.content = contents;
Expand Down
29 changes: 24 additions & 5 deletions test/jdk/tools/jpackage/share/InOutPathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,20 @@ public static Collection input() {
data.addAll(additionalContentInput(packageTypes, "--app-content"));
}

data.addAll(List.of(new Object[][]{
{PackageType.IMAGE.toString(), wrap(cmd -> {
additionalContent(cmd, "--app-content", cmd.outputBundle());
}, "--app-content same as output bundle")},
}));
if (!TKit.isOSX()) {
data.addAll(List.of(new Object[][]{
{PackageType.IMAGE.toString(), wrap(cmd -> {
additionalContent(cmd, "--app-content", cmd.outputBundle());
}, "--app-content same as output bundle")},
}));
} else {
var contentsFolder = "Contents/MacOS";
data.addAll(List.of(new Object[][]{
{PackageType.IMAGE.toString(), wrap(cmd -> {
additionalContent(cmd, "--app-content", cmd.outputBundle().resolve(contentsFolder));
}, String.format("--app-content same as the \"%s\" folder in the output bundle", contentsFolder))},
}));
}

if (TKit.isOSX()) {
data.addAll(additionalContentInput(PackageType.MAC_DMG.toString(),
Expand Down Expand Up @@ -172,6 +181,16 @@ private static void runTest(Set<PackageType> packageTypes,
if (isAppImageValid(cmd)) {
verifyAppImage(cmd);
}

if (cmd.hasArgument("--app-content")) {
// `--app-content` can be set to the app image directory which
// should not exist before jpackage is executed:
// jpackage --name Foo --dest output --app-content output/Foo
// Verify the directory exists after jpackage execution.
// At least this will catch the case when the value of
// `--app-content` option refers to a path unrelated to jpackage I/O.
TKit.assertDirectoryExists(Path.of(cmd.getArgumentValue("--app-content")));
}
} else {
new PackageTest()
.forTypes(packageTypes)
Expand Down

0 comments on commit ac82a8f

Please sign in to comment.