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

towards better module testing #3397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function(add_fmt_test name)
set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wno-weak-vtables)
endif ()
elseif (ADD_FMT_TEST_MODULE)
set(libs gtest test-module)
set(libs test-main test-module)
set_source_files_properties(${name}.cc PROPERTIES OBJECT_DEPENDS test-module)
else ()
set(libs test-main fmt)
Expand Down
7 changes: 1 addition & 6 deletions test/gtest-extra.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,7 @@

#include <string>

#ifdef FMT_MODULE_TEST
import fmt;
#else
# include "fmt/os.h"
#endif // FMG_MODULE_TEST

#include "fmt/os.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With all compilers having converged to the 'strong module ownership' model we no longer need to fold module fmt into the test machinery for module tests. The module has all its symbols attached to the module (e.g. void foo() becomes void foo@fmt()), and the test machinery can have the same definitions attached to the global module within the same program.

Therefore, the test-main library can be reused as with all other tests. And all stuff conditionally compiled by FMT_MODULE_TEST can kicked out again. Both Clang and MSVC are fine with it. Strong ownership rulez!

#include "gmock/gmock.h"

#define FMT_TEST_THROW_(statement, expected_exception, expected_message, fail) \
Expand Down
122 changes: 35 additions & 87 deletions test/module-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
# define FMT_HIDE_MODULE_BUGS
#endif

#define FMT_MODULE_TEST

#include <bit>
#include <chrono>
#include <exception>
Expand All @@ -41,7 +39,6 @@
#else
# define FMT_POSIX(call) call
#endif
#define FMT_OS_H_ // don't pull in os.h directly or indirectly

import fmt;

Expand All @@ -53,13 +50,8 @@ static bool macro_leaked =
false;
#endif

// Include sources to pick up functions and classes from the module rather than
// from the non-modular library which is baked into the 'test-main' library.
// This averts linker problems:
// - strong ownership model: missing linker symbols
// - weak ownership model: duplicate linker symbols
#include "gtest-extra.cc"
#include "util.cc"
#define FMT_OS_H_ // don't pull in os.h, neither directly nor indirectly
#include "gtest-extra.h"

// an implicitly exported namespace must be visible [module.interface]/2.2
TEST(module_test, namespace) {
Expand All @@ -75,9 +67,8 @@ bool oops_detail_namespace_is_visible;
namespace fmt {
bool namespace_detail_invisible() {
#if defined(FMT_HIDE_MODULE_BUGS) && defined(_MSC_FULL_VER) && \
((_MSC_VER == 1929 && _MSC_FULL_VER <= 192930136) || \
(_MSC_VER == 1930 && _MSC_FULL_VER <= 193030704))
// bug in msvc up to 16.11.5 / 17.0-pre5:
_MSC_FULL_VER <= 193700000
// bug in msvc up to at least 17.7:

// the namespace is visible even when it is neither
// implicitly nor explicitly exported
Expand Down Expand Up @@ -140,7 +131,7 @@ TEST(module_test, format_to) {
EXPECT_EQ("42", std::string_view(buffer));

fmt::memory_buffer mb;
fmt::format_to(mb, "{}", 42);
fmt::format_to(std::back_inserter(mb), "{}", 42);
EXPECT_EQ("42", std::string_view(buffer));

std::wstring w;
Expand All @@ -152,7 +143,7 @@ TEST(module_test, format_to) {
EXPECT_EQ(L"42", std::wstring_view(wbuffer));

fmt::wmemory_buffer wb;
fmt::format_to(wb, L"{}", 42);
fmt::format_to(std::back_inserter(wb), L"{}", 42);
EXPECT_EQ(L"42", std::wstring_view(wbuffer));
}

Expand Down Expand Up @@ -211,8 +202,8 @@ TEST(module_test, dynamic_format_args) {

TEST(module_test, vformat) {
EXPECT_EQ("42", fmt::vformat("{}", fmt::make_format_args(42)));
EXPECT_EQ(L"42", fmt::vformat(fmt::to_string_view(L"{}"),
fmt::make_wformat_args(42)));
EXPECT_EQ(L"42",
fmt::vformat(fmt::wstring_view(L"{}"), fmt::make_wformat_args(42)));
}

TEST(module_test, vformat_to) {
Expand Down Expand Up @@ -245,9 +236,9 @@ TEST(module_test, vformat_to_n) {
auto wstore = fmt::make_wformat_args(12345);
std::wstring w;
auto wresult = fmt::vformat_to_n(std::back_inserter(w), 1,
fmt::to_string_view(L"{}"), wstore);
fmt::wstring_view(L"{}"), wstore);
wchar_t wbuffer[4] = {0};
fmt::vformat_to_n(wbuffer, 3, fmt::to_string_view(L"{:}"), wstore);
fmt::vformat_to_n(wbuffer, 3, fmt::wstring_view(L"{:}"), wstore);
}

std::string as_string(std::wstring_view text) {
Expand All @@ -258,22 +249,18 @@ std::string as_string(std::wstring_view text) {
TEST(module_test, print) {
EXPECT_WRITE(stdout, fmt::print("{}µ", 42), "42µ");
EXPECT_WRITE(stderr, fmt::print(stderr, "{}µ", 4.2), "4.2µ");
if (false) {
EXPECT_WRITE(stdout, fmt::print(L"{}µ", 42), as_string(L"42µ"));
EXPECT_WRITE(stderr, fmt::print(stderr, L"{}µ", 4.2), as_string(L"4.2µ"));
}
EXPECT_WRITE(stdout, fmt::print(L"{}µ", 42), as_string(L"42µ"));
EXPECT_WRITE(stderr, fmt::print(stderr, L"{}µ", 4.2), as_string(L"4.2µ"));
}

TEST(module_test, vprint) {
EXPECT_WRITE(stdout, fmt::vprint("{:}µ", fmt::make_format_args(42)), "42µ");
EXPECT_WRITE(stderr, fmt::vprint(stderr, "{}", fmt::make_format_args(4.2)),
"4.2");
if (false) {
EXPECT_WRITE(stdout, fmt::vprint(L"{:}µ", fmt::make_wformat_args(42)),
as_string(L"42µ"));
EXPECT_WRITE(stderr, fmt::vprint(stderr, L"{}", fmt::make_wformat_args(42)),
as_string(L"42"));
}
EXPECT_WRITE(stdout, fmt::vprint(L"{:}µ", fmt::make_wformat_args(42)),
as_string(L"42µ"));
EXPECT_WRITE(stderr, fmt::vprint(stderr, L"{}", fmt::make_wformat_args(42)),
as_string(L"42"));
}

TEST(module_test, named_args) {
Expand All @@ -284,9 +271,7 @@ TEST(module_test, named_args) {
TEST(module_test, literals) {
using namespace fmt::literals;
EXPECT_EQ("42", fmt::format("{answer}", "answer"_a = 42));
EXPECT_EQ("42", "{}"_format(42));
EXPECT_EQ(L"42", fmt::format(L"{answer}", L"answer"_a = 42));
EXPECT_EQ(L"42", L"{}"_format(42));
}

TEST(module_test, locale) {
Expand Down Expand Up @@ -320,7 +305,7 @@ TEST(module_test, string_view) {

TEST(module_test, memory_buffer) {
fmt::basic_memory_buffer<char, fmt::inline_buffer_size> buffer;
fmt::format_to(buffer, "{}", "42");
fmt::format_to(std::back_inserter(buffer), "{}", "42");
EXPECT_EQ("42", to_string(buffer));
fmt::memory_buffer nbuffer(std::move(buffer));
EXPECT_EQ("42", to_string(nbuffer));
Expand Down Expand Up @@ -395,27 +380,20 @@ struct test_formatter : fmt::formatter<char> {
bool check() { return true; }
};

struct test_dynamic_formatter : fmt::dynamic_formatter<> {
bool check() { return true; }
};

TEST(module_test, formatter) {
EXPECT_TRUE(test_formatter{}.check());
EXPECT_TRUE(test_dynamic_formatter{}.check());
}
TEST(module_test, formatter) { EXPECT_TRUE(test_formatter{}.check()); }

TEST(module_test, join) {
int arr[3] = {1, 2, 3};
std::vector<double> vec{1.0, 2.0, 3.0};
std::initializer_list<int> il{1, 2, 3};
auto sep = fmt::to_string_view(", ");
auto sep = fmt::string_view(", ");
EXPECT_EQ("1, 2, 3", to_string(fmt::join(arr + 0, arr + 3, sep)));
EXPECT_EQ("1, 2, 3", to_string(fmt::join(arr, sep)));
EXPECT_EQ("1, 2, 3", to_string(fmt::join(vec.begin(), vec.end(), sep)));
EXPECT_EQ("1, 2, 3", to_string(fmt::join(vec, sep)));
EXPECT_EQ("1, 2, 3", to_string(fmt::join(il, sep)));

auto wsep = fmt::to_string_view(L", ");
auto wsep = fmt::wstring_view(L", ");
EXPECT_EQ(L"1, 2, 3", fmt::format(L"{}", fmt::join(arr + 0, arr + 3, wsep)));
EXPECT_EQ(L"1, 2, 3", fmt::format(L"{}", fmt::join(arr, wsep)));
EXPECT_EQ(L"1, 2, 3", fmt::format(L"{}", fmt::join(il, wsep)));
Expand All @@ -426,7 +404,6 @@ TEST(module_test, time) {
EXPECT_TRUE(fmt::localtime(time_now).tm_year > 120);
EXPECT_TRUE(fmt::gmtime(time_now).tm_year > 120);
auto chrono_now = std::chrono::system_clock::now();
EXPECT_TRUE(fmt::localtime(chrono_now).tm_year > 120);
EXPECT_TRUE(fmt::gmtime(chrono_now).tm_year > 120);
}

Expand All @@ -453,34 +430,16 @@ TEST(module_test, weekday) {
EXPECT_EQ("Mon", fmt::format(std::locale::classic(), "{}", fmt::weekday(1)));
}

TEST(module_test, to_string_view) {
using fmt::to_string_view;
fmt::string_view nsv{to_string_view("42")};
EXPECT_EQ("42", nsv);
fmt::wstring_view wsv{to_string_view(L"42")};
EXPECT_EQ(L"42", wsv);
}

TEST(module_test, printf) {
EXPECT_WRITE(stdout, fmt::printf("%f", 42.123456), "42.123456");
EXPECT_WRITE(stdout, fmt::printf("%d", 42), "42");
if (false) {
EXPECT_WRITE(stdout, fmt::printf(L"%f", 42.123456),
as_string(L"42.123456"));
EXPECT_WRITE(stdout, fmt::printf(L"%d", 42), as_string(L"42"));
}
EXPECT_WRITE(stdout, fmt::printf(L"%f", 42.123456), as_string(L"42.123456"));
EXPECT_WRITE(stdout, fmt::printf(L"%d", 42), as_string(L"42"));
}

TEST(module_test, fprintf) {
EXPECT_WRITE(stderr, fmt::fprintf(stderr, "%d", 42), "42");
std::ostringstream os;
fmt::fprintf(os, "%s", "bla");
EXPECT_EQ("bla", os.str());

EXPECT_WRITE(stderr, fmt::fprintf(stderr, L"%d", 42), as_string(L"42"));
std::wostringstream ws;
fmt::fprintf(ws, L"%s", L"bla");
EXPECT_EQ(L"bla", ws.str());
}

TEST(module_test, sprintf) {
Expand All @@ -490,25 +449,15 @@ TEST(module_test, sprintf) {

TEST(module_test, vprintf) {
EXPECT_WRITE(stdout, fmt::vprintf("%d", fmt::make_printf_args(42)), "42");
if (false) {
EXPECT_WRITE(stdout, fmt::vprintf(L"%d", fmt::make_wprintf_args(42)),
as_string(L"42"));
}
EXPECT_WRITE(stdout, fmt::vprintf(L"%d", fmt::make_wprintf_args(42)),
as_string(L"42"));
}

TEST(module_test, vfprintf) {
auto args = fmt::make_printf_args(42);
EXPECT_WRITE(stderr, fmt::vfprintf(stderr, "%d", args), "42");
std::ostringstream os;
fmt::vfprintf(os, "%d", args);
EXPECT_EQ("42", os.str());
auto wargs = fmt::make_wprintf_args(42);
if (false) {
EXPECT_WRITE(stderr, fmt::vfprintf(stderr, L"%d", wargs), as_string(L"42"));
}
std::wostringstream ws;
fmt::vfprintf(ws, L"%d", wargs);
EXPECT_EQ(L"42", ws.str());
EXPECT_WRITE(stderr, fmt::vfprintf(stderr, L"%d", wargs), as_string(L"42"));
}

TEST(module_test, vsprintf) {
Expand Down Expand Up @@ -538,8 +487,13 @@ TEST(module_test, buffered_file) {
}

TEST(module_test, output_file) {
#ifdef __clang__
fmt::println("\033[0;33m[=disabled=] {}\033[0;0m",
"Clang 16.0 emits multiple copies of vtables");
#else
fmt::ostream out = fmt::output_file("module-test", fmt::buffer_size = 1);
out.close();
#endif
}

struct custom_context {
Expand All @@ -552,21 +506,15 @@ TEST(module_test, custom_context) {
EXPECT_TRUE(!custom_arg);
}

struct disabled_formatter {};

TEST(module_test, has_formatter) {
EXPECT_FALSE(
(fmt::has_formatter<disabled_formatter, fmt::format_context>::value));
}

TEST(module_test, is_formattable) {
EXPECT_FALSE(fmt::is_formattable<disabled_formatter>::value);
}

TEST(module_test, compile_format_string) {
using namespace fmt::literals;
#ifdef __clang__
fmt::println("\033[0;33m[=disabled=] {}\033[0;0m",
"Clang 16.0 fails to import user-defined literals");
#else
EXPECT_EQ("42", fmt::format("{0:x}"_cf, 0x42));
EXPECT_EQ(L"42", fmt::format(L"{:}"_cf, 42));
EXPECT_EQ("4.2", fmt::format("{arg:3.1f}"_cf, "arg"_a = 4.2));
EXPECT_EQ(L" 42", fmt::format(L"{arg:>3}"_cf, L"arg"_a = L"42"));
#endif
}
6 changes: 1 addition & 5 deletions test/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@
#include <locale>
#include <string>

#ifdef FMT_MODULE_TEST
import fmt;
#else
# include "fmt/os.h"
#endif // FMT_MODULE_TEST
#include "fmt/os.h"

#ifdef _MSC_VER
# define FMT_VSNPRINTF vsprintf_s
Expand Down