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.

Sign up for GitHub

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

Support floats in spectest runner #445

Merged
merged 4 commits into from
Jul 31, 2020
Merged

Support floats in spectest runner #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
179d2e8
test: Support floating point arguments ad expected results in spectests
gumb0 Jul 31, 2020
e41dd73
test: update spectest smoke test with floating point supported
gumb0 Jul 31, 2020
4106bd9
ci: update spectest expectations
gumb0 Jul 31, 2020
8674884
test: smoke test for invalid value type
gumb0 Jul 31, 2020
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
24 changes: 12 additions & 12 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ jobs:
- benchmark:
min_time: "0.01"
- spectest:
expected_passed: 5468
expected_failed: 9
expected_skipped: 6336
expected_passed: 5577
expected_failed: 13
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

4 new failures are all in globals.wast, will be fixed in #446

expected_skipped: 6223

sanitizers-macos:
executor: macos
Expand All @@ -289,9 +289,9 @@ jobs:
- benchmark:
min_time: "0.01"
- spectest:
expected_passed: 5468
expected_failed: 9
expected_skipped: 6336
expected_passed: 5577
expected_failed: 13
expected_skipped: 6223

benchmark:
machine:
Expand Down Expand Up @@ -397,13 +397,13 @@ jobs:
at: ~/build
- spectest:
skip_validation: true
expected_passed: 4526
expected_failed: 9
expected_skipped: 7278
expected_passed: 4635
expected_failed: 13
expected_skipped: 7165
- spectest:
expected_passed: 5468
expected_failed: 9
expected_skipped: 6336
expected_passed: 5577
expected_failed: 13
expected_skipped: 6223
- collect_coverage_data

workflows:
Expand Down
6 changes: 3 additions & 3 deletions test/smoketests/spectests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ add_test(
set_tests_properties(
fizzy/smoketests/spectests/default
PROPERTIES
PASS_REGULAR_EXPRESSION "PASSED 23, FAILED 0, SKIPPED 7"
PASS_REGULAR_EXPRESSION "PASSED 26, FAILED 1, SKIPPED 3"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1 failure is for float global, will be fixed in #446

)

add_test(
Expand All @@ -19,7 +19,7 @@ add_test(
set_tests_properties(
fizzy/smoketests/spectests/skipvalidation
PROPERTIES
PASS_REGULAR_EXPRESSION "PASSED 22, FAILED 0, SKIPPED 8"
PASS_REGULAR_EXPRESSION "PASSED 25, FAILED 1, SKIPPED 4"
)

add_test(
Expand All @@ -39,7 +39,7 @@ add_test(
set_tests_properties(
fizzy/smoketests/spectests/broken
PROPERTIES
PASS_REGULAR_EXPRESSION "PASSED 0, FAILED 2, SKIPPED 4"
PASS_REGULAR_EXPRESSION "PASSED 1, FAILED 2, SKIPPED 6"
)

add_test(
Expand Down
6 changes: 5 additions & 1 deletion test/smoketests/spectests/broken/broken.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@
{"type": "module", "line": 5, "filename": "unparsable.wasm"},
{"type": "assert_return", "line": 10, "action": {"type": "get", "field": "tmp"}, "expected": []},
{"type": "assert_return", "line": 15, "action": {"type": "invalid_action_type"}, "expected": []},
{"type": "assert_trap", "line": 20, "action": {"type": "invalid_action_type"}, "expected": []}]}
{"type": "assert_trap", "line": 20, "action": {"type": "invalid_action_type"}, "expected": []},
{"type": "module", "line": 25, "filename": "single_func.wasm"},
{"type": "assert_return", "line": 35, "action": {"type": "invoke", "field": "foo", "args": [{"type": "invalid_type", "value": "0"}]}, "expected": []},
{"type": "assert_return", "line": 45, "action": {"type": "invoke", "field": "foo", "args": []}, "expected": [{"type": "invalid_type", "value": "0"}]}]
}
Binary file added test/smoketests/spectests/broken/single_func.wasm
View file
Open in desktop
Binary file not shown.
10 changes: 5 additions & 5 deletions test/smoketests/spectests/default/smoketest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
{"type": "assert_unlinkable", "line": 72, "filename": "smoketest.9.wasm", "text": "unknown import", "module_type": "binary"},
{"type": "assert_unlinkable", "line": 76, "filename": "smoketest.10.wasm", "text": "data segment does not fit", "module_type": "binary"},
{"type": "assert_uninstantiable", "line": 81, "filename": "smoketest.11.wasm", "text": "unreachable", "module_type": "binary"},
{"type": "module", "line": 88, "filename": "smoketest.12.wasm"},
{"type": "assert_return", "line": 95, "action": {"type": "invoke", "field": "foo.f32", "args": []}, "expected": [{"type": "f32", "value": "1067282596"}]},
{"type": "assert_return", "line": 96, "action": {"type": "invoke", "field": "foo.f64", "args": []}, "expected": [{"type": "f64", "value": "4616820122002590269"}]},
{"type": "action", "line": 97, "action": {"type": "invoke", "field": "param.f64", "args": [{"type": "f64", "value": "4607182418800017408"}]}, "expected": []},
{"type": "assert_return", "line": 98, "action": {"type": "get", "field": "glob.f32"}, "expected": [{"type": "f32", "value": "1085276160"}]},
{"type": "module", "line": 86, "filename": "smoketest.12.wasm"},
{"type": "assert_return", "line": 93, "action": {"type": "invoke", "field": "foo.f32", "args": []}, "expected": [{"type": "f32", "value": "1067282596"}]},
{"type": "assert_return", "line": 94, "action": {"type": "invoke", "field": "foo.f64", "args": []}, "expected": [{"type": "f64", "value": "4616820122002590269"}]},
{"type": "action", "line": 95, "action": {"type": "invoke", "field": "param.f64", "args": [{"type": "f64", "value": "4607182418800017408"}]}, "expected": []},
{"type": "assert_return", "line": 96, "action": {"type": "get", "field": "glob.f32"}, "expected": [{"type": "f32", "value": "1085276160"}]},
{"type": "register", "line": 100, "name": "$Mod-unknown", "as": "Mod-unknown"},
{"type": "assert_malformed", "line": 103, "filename": "smoketest.13.wat", "text": "error", "module_type": "text"},
{"type": "assert_unlinkable", "line": 104, "filename": "smoketest.14.wat", "text": "error", "module_type": "text"}]}
4 changes: 2 additions & 2 deletions test/smoketests/spectests/default/smoketest.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@
"unreachable"
)

;; cases that will be skipped

;; floating point module
(module
(func (export "foo.f32") (result f32) (f32.const 1.23))
Expand All @@ -97,6 +95,8 @@
(invoke "param.f64" (f64.const 1))
(assert_return (get "glob.f32") (f32.const 5.5))

;; cases that will be skipped

(register "Mod-unknown" $Mod-unknown)

;; module_type=text
Expand Down
55 changes: 24 additions & 31 deletions test/spectests/spectests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ const auto spectest_bin = fizzy::test::from_hex(
const auto spectest_module = fizzy::parse(spectest_bin);
const std::string spectest_name = "spectest";

template <typename T>
uint64_t json_to_value(const json& v)
{
return static_cast<std::make_unsigned_t<T>>(std::stoull(v.get<std::string>()));
}

fizzy::bytes load_wasm_file(const fs::path& json_file_path, std::string_view filename)
{
std::ifstream wasm_file{fs::path{json_file_path}.replace_filename(filename)};
Expand Down Expand Up @@ -402,6 +396,20 @@ class test_runner
return it_instance->second.get();
}

std::optional<fizzy::Value> read_value(const json& v)
{
const auto arg_type = v.at("type").get<std::string>();
if (arg_type != "i32" && arg_type != "i64" && arg_type != "f32" && arg_type != "f64")
{
skip("Unsupported value type '" + arg_type + "'.");
Copy link
Member

Choose a reason for hiding this comment

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

This is not covered by unit tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests.

return std::nullopt;
}

// Values of all types are serialized to JSON as integers.
// Value type will handle correct conversions.
Copy link
Member

Choose a reason for hiding this comment

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

How? Why not keep the old json_to_value<int32_t>(arg.at("value")) way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's just simpler without additional switch and template.

Presumably wast2json does the opposite conversion - typed value into unsigned integer. We could reinterpret_cast it back to correct type, but Value(correct_type_value) constructor does the same as Value(reinterpret_cast<unsigned long long>(correct_type_value)), which is the idea of Value type...

return std::stoull(v.at("value").get<std::string>());
}

std::optional<fizzy::ExecutionResult> invoke(const json& action)
{
auto instance = find_instance_for_action(action);
Expand All @@ -419,18 +427,11 @@ class test_runner
std::vector<fizzy::Value> args;
for (const auto& arg : action.at("args"))
{
const auto arg_type = arg.at("type").get<std::string>();
uint64_t arg_value;
if (arg_type == "i32")
arg_value = json_to_value<int32_t>(arg.at("value"));
else if (arg_type == "i64")
arg_value = json_to_value<int64_t>(arg.at("value"));
else
{
skip("Unsupported argument type '" + arg_type + "'.");
const auto arg_value = read_value(arg);
if (!arg_value.has_value())
return std::nullopt;
}
args.push_back(arg_value);

args.push_back(*arg_value);
}

try
Expand All @@ -444,25 +445,17 @@ class test_runner
}
}

bool check_result(uint64_t actual_value, const json& expected)
bool check_result(fizzy::Value actual_value, const json& expected)
{
const auto expected_type = expected.at("type").get<std::string>();
uint64_t expected_value;
if (expected_type == "i32")
expected_value = json_to_value<int32_t>(expected.at("value"));
else if (expected_type == "i64")
expected_value = json_to_value<int64_t>(expected.at("value"));
else
{
skip("Unsupported expected type '" + expected_type + "'.");
const auto expected_value = read_value(expected);
if (!expected_value.has_value())
return false;
}

if (expected_value != actual_value)
if (*expected_value != actual_value)
{
std::stringstream message;
message << "Incorrect returned value. Expected: " << expected_value << " (0x"
<< std::hex << expected_value << ") Actual: " << std::dec << actual_value
message << "Incorrect returned value. Expected: " << *expected_value << " (0x"
<< std::hex << *expected_value << ") Actual: " << std::dec << actual_value
<< " (0x" << std::hex << actual_value << std::dec << ")";
fail(message.str());
return false;
Expand Down