Skip to content

Commit

Permalink
Include non-directories in `DirectoryListingHelper::leafDirectoryEntr…
Browse files Browse the repository at this point in the history
…ies` result.

`DirectoryListingHelper::leafDirectoryEntries` has a bug that it will only
include empty directories in the result. Fix the helper to include
non-directory entries and add unit tests covering that.

PiperOrigin-RevId: 376977471
  • Loading branch information
alexjski authored and copybara-github committed Jun 2, 2021
1 parent e49a69e commit 6b8eabb
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public final class DirectoryListingHelper {

private DirectoryListingHelper() {}

/** Shorthand for {@link Dirent} of {@link Dirent.Type#FILE} type with a given name. */
public static Dirent file(String name) {
return new Dirent(name, Dirent.Type.FILE);
}

/** Shorthand for {@link Dirent} of {@link Dirent.Type#DIRECTORY} type with a given name. */
public static Dirent directory(String name) {
return new Dirent(name, Dirent.Type.DIRECTORY);
Expand Down Expand Up @@ -52,16 +57,19 @@ private static void leafDirectoryEntriesInternal(
Path path, String prefix, ImmutableList.Builder<Dirent> entries) throws IOException {
boolean isEmpty = true;
for (Dirent dirent : path.readdir(Symlinks.NOFOLLOW)) {
isEmpty = false;
String entryName = prefix.isEmpty() ? dirent.getName() : prefix + "/" + dirent.getName();

if (dirent.getType() == Dirent.Type.DIRECTORY) {
leafDirectoryEntriesInternal(
path.getChild(dirent.getName()),
prefix.isEmpty() ? dirent.getName() : prefix + "/" + dirent.getName(),
entries);
leafDirectoryEntriesInternal(path.getChild(dirent.getName()), entryName, entries);
continue;
}
isEmpty = false;

entries.add(new Dirent(entryName, dirent.getType()));
}

if (isEmpty) {
// Skip adding the root if it's empty.
if (isEmpty && !prefix.isEmpty()) {
entries.add(new Dirent(prefix, Dirent.Type.DIRECTORY));
}
}
Expand Down
16 changes: 14 additions & 2 deletions src/test/java/com/google/devtools/build/lib/testing/common/BUILD
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
load("@rules_java//java:defs.bzl", "java_test")

package(
default_testonly = 1,
default_testonly = True,
default_visibility = ["//src:__subpackages__"],
)

licenses(["notice"])

filegroup(
name = "srcs",
testonly = 0,
testonly = False,
srcs = glob(["**"]),
visibility = ["//src:__subpackages__"],
)

java_test(
name = "DirectoryListingHelperTest",
srcs = ["DirectoryListingHelperTest.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/testing/common:directory_listing_helper",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/test/java/com/google/devtools/build/lib/testutil",
"//third_party:junit4",
"//third_party:truth",
],
)

java_test(
name = "FakeOptionsTest",
srcs = ["FakeOptionsTest.java"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// Copyright 2021 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.testing.common;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testing.common.DirectoryListingHelper.directory;
import static com.google.devtools.build.lib.testing.common.DirectoryListingHelper.file;
import static com.google.devtools.build.lib.testing.common.DirectoryListingHelper.leafDirectoryEntries;
import static org.junit.Assert.assertThrows;

import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Path;
import java.io.FileNotFoundException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Unit tests for {@link com.google.devtools.build.lib.testing.common.DirectoryListingHelper}. */
@RunWith(JUnit4.class)
public final class DirectoryListingHelperTest {

private final Scratch scratch = new Scratch();
private final Path root = scratch.getFileSystem().getPath("/");

@Test
public void leafDirectoryEntries_emptyDirectory_returnsEmptyList() throws Exception {
assertThat(leafDirectoryEntries(root)).isEmpty();
}

@Test
public void leafDirectoryEntries_returnsFile() throws Exception {
scratch.file("/file");
assertThat(leafDirectoryEntries(root)).containsExactly(file("file"));
}

@Test
public void leafDirectoryEntries_fileInSubfolders_returnsFileOnly() throws Exception {
scratch.file("/dir1/dir2/file");
assertThat(leafDirectoryEntries(root)).containsExactly(file("dir1/dir2/file"));
}

@Test
public void leafDirectoryEntries_returnsEmptyDirectory() throws Exception {
scratch.dir("/dir");
assertThat(leafDirectoryEntries(root)).containsExactly(directory("dir"));
}

@Test
public void leafDirectoryEntries_mixedEmptyDirectoriesAndFiles_returnsAllEntries()
throws Exception {
scratch.dir("/dir/empty1");
scratch.dir("/dir/subdir/empty2");
scratch.file("/dir2/file3");
scratch.file("/dir2/file4");

assertThat(leafDirectoryEntries(root))
.containsExactly(
directory("dir/empty1"),
directory("dir/subdir/empty2"),
file("dir2/file3"),
file("dir2/file4"));
}

@Test
public void leafDirectoryEntries_returnsEntriesUnderProvidedPathOnly() throws Exception {
scratch.file("/dir/file1");
scratch.file("/dir2/file2");
Path dir = scratch.dir("/dir");

assertThat(leafDirectoryEntries(dir)).containsExactly(file("file1"));
}

@Test
public void leafDirectoryEntries_missingDirectory_fails() {
Path nonexistent = scratch.getFileSystem().getPath("/nonexistent");
assertThrows(FileNotFoundException.class, () -> leafDirectoryEntries(nonexistent));
}
}

0 comments on commit 6b8eabb

Please sign in to comment.