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

Add Bazel tests #1601

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
33 changes: 29 additions & 4 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,29 @@
licenses(["unencumbered"]) # Public Domain or MIT

load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")

exports_files(["LICENSE"])

bool_flag(
name = "use_exception",
build_setting_default = False,
Copy link
Contributor

Choose a reason for hiding this comment

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

Exceptions should be on by default, I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an abstract question, that's debatable (as in there are good arguments either way). But, given that the existing Bazel build has them off by default, I think backwards compatibility requires keeping them that way.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also: probably a discussion that should be on the record of #1601 (unless you want to merge just this PR and not that one).

)

config_setting(
name = "use_exception_cfg",
flag_values = {":use_exception": "true"},
)

bool_flag(
name = "has_int64",
build_setting_default = True,
)

config_setting(
name = "has_int64_cfg",
flag_values = {":has_int64": "true"},
)

cc_library(
name = "jsoncpp",
srcs = [
Expand All @@ -22,10 +44,13 @@ cc_library(
"include/json/version.h",
"include/json/writer.h",
],
copts = [
"-DJSON_USE_EXCEPTION=0",
"-DJSON_HAS_INT64",
],
defines = select({
":use_exception_cfg": ["JSON_USE_EXCEPTION=1"],
"//conditions:default": ["JSON_USE_EXCEPTION=0"],
}) + select({
":has_int64_cfg": ["JSON_HAS_INT64"],
"//conditions:default": [],
}),
includes = ["include"],
visibility = ["//visibility:public"],
deps = [":private"],
Expand Down
5 changes: 5 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ module(
version = "1.9.7",
compatibility_level = 1,
)

bazel_dep(
name = "bazel_skylib",
version = "1.7.1",
)
6 changes: 6 additions & 0 deletions src/jsontestrunner/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
cc_binary(
name = "jsontestrunner",
srcs = ["main.cpp"],
deps = ["//:jsoncpp"],
visibility = ["//test:__pkg__"],
)
11 changes: 11 additions & 0 deletions src/test_lib_json/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
cc_test(
name = "jsoncpp_test",
srcs = [
"jsontest.cpp",
"jsontest.h",
"main.cpp",
"fuzz.h",
"fuzz.cpp",
],
deps = ["//:jsoncpp"],
)
8 changes: 4 additions & 4 deletions src/test_lib_json/fuzz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
#include <memory>
#include <string>

namespace Json {
class Exception;
}

extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
Json::CharReaderBuilder builder;

Expand Down Expand Up @@ -45,10 +41,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {

Json::Value root;
const auto data_str = reinterpret_cast<const char*>(data);
#if JSON_USE_EXCEPTION
try {
#endif // JSON_USE_EXCEPTION
reader->parse(data_str, data_str + size, &root, nullptr);
#if JSON_USE_EXCEPTION
} catch (Json::Exception const&) {
}
#endif // JSON_USE_EXCEPTION
// Whether it succeeded or not doesn't matter.
return 0;
}
4 changes: 4 additions & 0 deletions src/test_lib_json/jsontest.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ TestResult& checkStringEqual(TestResult& result, const Json::String& expected,
JsonTest::ToJsonString(actual), __FILE__, \
__LINE__, #expected " == " #actual)

#if JSON_USE_EXCEPTION

/// \brief Asserts that a given expression throws an exception
#define JSONTEST_ASSERT_THROWS(expr) \
do { \
Expand All @@ -242,6 +244,8 @@ TestResult& checkStringEqual(TestResult& result, const Json::String& expected,
"expected exception thrown: " #expr); \
} while (0)

#endif // JSON_USE_EXCEPTION

/// \brief Begin a fixture test case.
#define JSONTEST_FIXTURE(FixtureType, name) \
class Test##FixtureType##name : public FixtureType { \
Expand Down
10 changes: 9 additions & 1 deletion src/test_lib_json/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1888,7 +1888,7 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, typeChecksThrowExceptions) {
JSONTEST_ASSERT_THROWS(objVal.asBool());
JSONTEST_ASSERT_THROWS(arrVal.asBool());

#endif
#endif // JSON_USE_EXCEPTION
}

JSONTEST_FIXTURE_LOCAL(ValueTest, offsetAccessors) {
Expand Down Expand Up @@ -3323,6 +3323,8 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithDetailError) {
}

JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) {
#if JSON_USE_EXCEPTION

Json::CharReaderBuilder b;
Json::Value root;
char const doc[] = R"({ "property" : "value" })";
Expand All @@ -3342,6 +3344,8 @@ JSONTEST_FIXTURE_LOCAL(CharReaderTest, parseWithStackLimit) {
JSONTEST_ASSERT_THROWS(
reader->parse(doc, doc + std::strlen(doc), &root, &errs));
}

#endif // JSON_USE_EXCEPTION
}

JSONTEST_FIXTURE_LOCAL(CharReaderTest, testOperator) {
Expand Down Expand Up @@ -3961,6 +3965,8 @@ JSONTEST_FIXTURE_LOCAL(IteratorTest, indexes) {
}

JSONTEST_FIXTURE_LOCAL(IteratorTest, constness) {
#if JSON_USE_EXCEPTION

Json::Value const v;
JSONTEST_ASSERT_THROWS(
Json::Value::iterator it(v.begin())); // Compile, but throw.
Expand All @@ -3982,6 +3988,8 @@ JSONTEST_FIXTURE_LOCAL(IteratorTest, constness) {
}
Json::String expected = R"(" 9","10","11",)";
JSONTEST_ASSERT_STRING_EQUAL(expected, out.str());

#endif // JSON_USE_EXCEPTION
}

struct RValueTest : JsonTest::TestCase {};
Expand Down
20 changes: 20 additions & 0 deletions test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
filegroup(
name = "expected",
srcs = glob(["data/**", "jsonchecker/**"], exclude=["**/*.json"]),
)

[py_test(
name = "runjson_%s_test" % "_".join(f.split("/")),
srcs = ["runjsontests.py"],
main = "runjsontests.py",
args = [
"--with-json-checker",
"$(location //src/jsontestrunner:jsontestrunner)",
"$(location :%s)" % f,
],
data = [
"//src/jsontestrunner:jsontestrunner",
":expected",
":%s" % f,
],
) for f in glob(["**/*.json"])]
61 changes: 37 additions & 24 deletions test/runjsontests.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,38 +66,51 @@ class FailError(Exception):
def __init__(self, msg):
super(Exception, self).__init__(msg)

def runAllTests(jsontest_executable_path, input_dir = None,
def runAllTests(jsontest_executable_path, input_path = None,
use_valgrind=False, with_json_checker=False,
writerClass='StyledWriter'):
if not input_dir:
input_dir = os.path.join(os.getcwd(), 'data')
tests = glob(os.path.join(input_dir, '*.json'))
if with_json_checker:
all_tests = glob(os.path.join(input_dir, '../jsonchecker', '*.json'))
# These tests fail with strict json support, but pass with JsonCPP's
# extra leniency features. When adding a new exclusion to this list,
# remember to add the test's number and reasoning here:
known = ["fail{}.json".format(n) for n in [
4, 9, # fail because we allow trailing commas
7, # fails because we allow commas after close
8, # fails because we allow extra close
10, # fails because we allow extra values after close
13, # fails because we allow leading zeroes in numbers
18, # fails because we allow deeply nested values
25, # fails because we allow tab characters in strings
27, # fails because we allow string line breaks
]]
test_jsonchecker = [ test for test in all_tests
if os.path.basename(test) not in known]
if not input_path:
input_path = os.path.join(os.getcwd(), 'data')

if os.path.isdir(input_path):
tests = [
os.path.normpath(os.path.abspath(test))
for test in glob(os.path.join(input_path, '*.json'))
]

if with_json_checker:
tests += [
os.path.normpath(os.path.abspath(test))
for test in glob(os.path.join(input_path, '../jsonchecker', '*.json'))
]
else:
test_jsonchecker = []
tests = [input_path]

# These tests fail with strict json support, but pass with JsonCPP's
# extra leniency features. When adding a new exclusion to this list,
# remember to add the test's number and reasoning here:
known = ["fail{}.json".format(n) for n in [
4, 9, # fail because we allow trailing commas
7, # fails because we allow commas after close
8, # fails because we allow extra close
10, # fails because we allow extra values after close
13, # fails because we allow leading zeroes in numbers
18, # fails because we allow deeply nested values
25, # fails because we allow tab characters in strings
27, # fails because we allow string line breaks
]]

tests = [
test for test in tests
if os.path.basename(test) not in known or
os.path.basename(os.path.dirname(test)) != "jsonchecker"
]

failed_tests = []
valgrind_path = use_valgrind and VALGRIND_CMD or ''
for input_path in tests + test_jsonchecker:
for input_path in tests:
expect_failure = os.path.basename(input_path).startswith('fail')
is_json_checker_test = input_path in test_jsonchecker
is_json_checker_test = os.path.basename(os.path.dirname(input_path)) == "jsonchecker"
is_parse_only = is_json_checker_test or expect_failure
is_strict_test = ('_strict_' in os.path.basename(input_path)) or is_json_checker_test
print('TESTING:', input_path, end=' ')
Expand Down
Loading