-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add minimal framework for spec test #46
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
base: main
Are you sure you want to change the base?
Conversation
5335efd
to
1980e30
Compare
} | ||
|
||
if (comptime isFixedType(ST)) { | ||
try ST.deserializeFromBytes(serialized, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try ST.deserializeFromBytes(serialized, value); | |
try ST.deserializeFromBytes(serialized, &@field(out, fld.name)); |
hence no need to declare value here
we can simplify "*Out" not to contain pointers, for example
pub const Phase0OperationsOut = struct {
pre: ?ssz.phase0.BeaconState.Type = null,
post: ?ssz.phase0.BeaconState.Type = null,
attestation: ?ssz.phase0.Attestation.Type = null,
attester_slashing: ?ssz.phase0.AttesterSlashing.Type = null,
block: ?ssz.phase0.BeaconBlock.Type = null,
deposit: ?ssz.phase0.Deposit.Type = null,
proposer_slashing: ?ssz.phase0.ProposerSlashing.Type = null,
voluntary_exit: ?ssz.phase0.SignedVoluntaryExit.Type = null,
};
and we don't need an inline for
to deinit fields at consumer side https://github.com/ChainSafe/state-transition-z/pull/46/files#diff-1d6f5b6eb82151a80442a6c728e68021970e2170f6ab4bb3574ac4f44b10886fR27
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@twoeths The thing is each field has default value of null. So &@field(out, fld.name)
is a pointer to a nullable, which ST.deserializeFromBytes
doesn't accept.
We need to pass a ST.Type pointer to ST.deserializeFromBytes
by creating one, and then assign the field to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general approach looks good to me, left some suggestions to deduplicate schemas and simplify things a bit, let me know if it does not work
const DIM = "\x1b[2m"; | ||
const RESET = "\x1b[0m"; | ||
|
||
pub const TestLogger = struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you using this TestLogger to run the actual test? Instead of just calling runTestCase
directly inside the test body?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a global object to capture the result of every tests and report the overall statistics at the very end of test execution.
The operations_tests.zig
looks something like this. Notice the TL.summary
at the end.
// This file is generated by write_static_tests.zig.
// Do not commit changes by hand.
const std = @import("std");
const types = @import("consensus_types");
const ForkSeq = @import("config").ForkSeq;
const OperationsTestHandler = @import("../test_type/handler.zig").OperationsTestHandler;
const test_case = @import("./operations.zig");
var TL = @import("../utils/test_logger.zig").TestLogger{};
const allocator = std.heap.page_allocator;
test "Static - phase0 operations proposer_slashing invalid_slots_of_different_epochs" {
try TL.run();
...
}
test "Static - phase0 operations proposer_slashing invalid_incorrect_sig_2" {
try TL.run();
...
}
test "__summary__(Operations Tests)" {
TL.summary("Operations Tests");
}
How to run
zig build run:download_spec_tests && zig run:write_spec_tests && zig test:spec_tests
Sample result
Part of #21