Skip to content

Commit

Permalink
encoder_main_lib_test: Prefer GetAndCreateOutputDirectory() over …
Browse files Browse the repository at this point in the history
…`std::filesystem::temp_directory_path()`.

  - Prefer `GetAndCreateOutputDirectory` which automatically creates a clean directory unique to the test case.
    - Conforms to codebase style which strongly prefers this function in tests.
    - Ensure tests are independent and work from a clean state.
  - Avoid creating directories when it is irrelevant to the test.
  - b/314895932: Remove unneeded use of `/dev/null/`. Writing to an empty sink behavior is already triggered when `IamfComponents` encounters an empty `file_name_prefix` (guarded by `EncoderMainLibTest/SettingPrefixOutputsFile`).

PiperOrigin-RevId: 673000182
  • Loading branch information
jwcullen committed Sep 10, 2024
1 parent 95bf210 commit 3ca6424
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 29 deletions.
1 change: 1 addition & 0 deletions iamf/cli/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ cc_test(
],
shard_count = 16,
deps = [
":cli_test_utils",
"//iamf/cli:encoder_main_lib",
"//iamf/cli/proto:codec_config_cc_proto",
"//iamf/cli/proto:ia_sequence_header_cc_proto",
Expand Down
48 changes: 19 additions & 29 deletions iamf/cli/tests/encoder_main_lib_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
#include "iamf/cli/proto/ia_sequence_header.pb.h"
#include "iamf/cli/proto/test_vector_metadata.pb.h"
#include "iamf/cli/proto/user_metadata.pb.h"
#include "iamf/cli/tests/cli_test_utils.h"
#include "src/google/protobuf/text_format.h"

namespace iamf_tools {
namespace {

using ::absl_testing::IsOk;
// TODO(b/314895932): Find a more portable alternative to `/dev/null/`.
constexpr absl::string_view kOutputIamfToDevNull = "/dev/null";
constexpr absl::string_view kIgnoredOutputPath = "";

void AddIaSequenceHeader(iamf_tools_cli_proto::UserMetadata& user_metadata) {
ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(
Expand Down Expand Up @@ -75,16 +75,14 @@ TEST(EncoderMainLibTest, IaSequenceHeaderOnly) {
// `partition_mix_gain_parameter_blocks` is left true (the default value).
iamf_tools_cli_proto::UserMetadata user_metadata;
AddIaSequenceHeader(user_metadata);
EXPECT_FALSE(TestMain(user_metadata, "",
std::filesystem::temp_directory_path().string())
.ok());
EXPECT_FALSE(
TestMain(user_metadata, "", std::string(kIgnoredOutputPath)).ok());

// After setting `partition_mix_gain_parameter_blocks` to false, `TestMain()`
// will succeed.
user_metadata.mutable_test_vector_metadata()
->set_partition_mix_gain_parameter_blocks(false);
EXPECT_THAT(TestMain(user_metadata, "",
std::filesystem::temp_directory_path().string()),
EXPECT_THAT(TestMain(user_metadata, "", std::string(kIgnoredOutputPath)),
IsOk());
}

Expand All @@ -94,8 +92,7 @@ TEST(EncoderMainLibTest, IaSequenceHeaderAndCodecConfigSucceeds) {
iamf_tools_cli_proto::UserMetadata user_metadata;
AddIaSequenceHeader(user_metadata);
AddCodecConfig(user_metadata);
EXPECT_THAT(TestMain(user_metadata, "",
std::filesystem::temp_directory_path().string()),
EXPECT_THAT(TestMain(user_metadata, "", std::string(kIgnoredOutputPath)),
IsOk());
}

Expand All @@ -110,8 +107,7 @@ TEST(EncoderMainLibTest, ConfigureOutputWavFileBitDepthOverrideSucceeds) {
user_metadata.mutable_test_vector_metadata()
->set_output_wav_file_bit_depth_override(16);

EXPECT_THAT(TestMain(user_metadata, "",
std::filesystem::temp_directory_path().string()),
EXPECT_THAT(TestMain(user_metadata, "", std::string(kIgnoredOutputPath)),
IsOk());
}

Expand All @@ -126,9 +122,8 @@ TEST(EncoderMainLibTest, ConfigureOutputWavFileBitDepthOverrideTooHighFails) {
user_metadata.mutable_test_vector_metadata()
->set_output_wav_file_bit_depth_override(kBitDepthTooHigh);

EXPECT_FALSE(TestMain(user_metadata, "",
std::filesystem::temp_directory_path().string())
.ok());
EXPECT_FALSE(
TestMain(user_metadata, "", std::string(kIgnoredOutputPath)).ok());
}

TEST(EncoderMainLibTest, SettingPrefixOutputsFile) {
Expand All @@ -140,12 +135,12 @@ TEST(EncoderMainLibTest, SettingPrefixOutputsFile) {
// Setting a filename prefix makes the function output a .iamf file.
user_metadata.mutable_test_vector_metadata()->set_file_name_prefix("empty");

const auto output_iamf_directory = std::filesystem::temp_directory_path();
const auto output_iamf_directory = GetAndCreateOutputDirectory("");

EXPECT_THAT(TestMain(user_metadata, "", output_iamf_directory.string()),
IsOk());
EXPECT_THAT(TestMain(user_metadata, "", output_iamf_directory), IsOk());

EXPECT_TRUE(std::filesystem::exists(output_iamf_directory / "empty.iamf"));
EXPECT_TRUE(std::filesystem::exists(
std::filesystem::path(output_iamf_directory) / "empty.iamf"));
}

TEST(EncoderMainLibTest, CreatesAndWritesToOutputIamfDirectory) {
Expand All @@ -157,16 +152,11 @@ TEST(EncoderMainLibTest, CreatesAndWritesToOutputIamfDirectory) {
// Setting a filename prefix makes the function output a .iamf file.
user_metadata.mutable_test_vector_metadata()->set_file_name_prefix("empty");

// Create a clean output directory.
const auto test_directory_root = GetAndCreateOutputDirectory("");

// The encoder will create and write the file based on a (nested)
// `output_iamf_directory` argument.
const auto test_directory_root =
std::filesystem::temp_directory_path() /
std::filesystem::path("encoder_main_lib_test");

// Clean up any previously created file and directories.
std::filesystem::remove_all(test_directory_root.c_str());
ASSERT_FALSE(std::filesystem::exists(test_directory_root));

const auto output_iamf_directory =
test_directory_root / std::filesystem::path("EncoderMainLibTest") /
std::filesystem::path("CreatesAndWritesToOutputIamfDirectory");
Expand Down Expand Up @@ -210,13 +200,13 @@ TEST_P(TestVector, ValidateTestSuite) {
ParseUserMetadataAssertSuccess(user_metadata_filename.string(),
user_metadata);

// Call encoder. Clear `file_name_prefix` and set all output files to
// "/dev/null"; we only care about the status.
// Call encoder. Clear `file_name_prefix`; we only care about the status and
// not the output files.
user_metadata.mutable_test_vector_metadata()->clear_file_name_prefix();
LOG(INFO) << "Testing with " << test_case.textproto_filename;
const absl::Status result =
iamf_tools::TestMain(user_metadata, input_wav_dir.string().c_str(),
std::string(kOutputIamfToDevNull));
std::string(kIgnoredOutputPath));

// Check if the result matches the expected value in the protos.
if (user_metadata.test_vector_metadata().is_valid()) {
Expand Down

0 comments on commit 3ca6424

Please sign in to comment.