test: add ethdebug/conformance#215
Conversation
|
cc @gnidan |
gnidan
left a comment
gnidan
left a comment
There was a problem hiding this comment.
This conformance suite is awesome! Thank you for putting it together @djolertrk 🙏
Mostly I'm requesting changes to make sure we test against the JSON-Schemas themselves, but maybe you'll agree with my other comments also. Let me know if you run into trouble with the hyperjump stuff.
On a broader note, this stuff might get a bit complicated when we want to add schema versioning, since then suddenly the conformance tests have to grow in dimensionality... Have you thought about this at all? Obviously that conversation remains a bit premature [and definitely shouldn't block getting this PR merged]
| } | ||
|
|
||
| artifact.programs.forEach((program, index) => { | ||
| if (!isProgram(program.program)) { |
There was a problem hiding this comment.
This (and the similar Materials.isCompilation checks) validate against the hand-written type guards, but nothing here checks against the JSON-Schemas themselves.
Type guards will always drift from the schema, of course, and in this instance, there's likely to be many false positives around things like unevaluatedProperties: false, use of if/then/else for discriminators... probably even things like regex matching with pattern I'd bet is pretty incomplete in the TS.
So yeah, heh, for a conformance test, we definitely want to validate against the most normative thing. Of course, it doesn't hurt to keep these type guard checks also!
Hopefully it should be pretty easy to validate against the schemas... this repo already does schema validation here, using a vitest guard defined here.
| } | ||
|
|
||
| describe("@ethdebug/conformance", () => { | ||
| it("[bug] compiles BUG fixtures into valid ETHDebug programs", async () => { |
There was a problem hiding this comment.
We probably want to also check against negative fixtures too. Nothing too complicated, but we do want to prove that the validator itself does validate. Maybe we can just feed it some deliberately malformed examples to make sure that result.ok comes back false when it should.
| types: {}, | ||
| pointers: {}, |
There was a problem hiding this comment.
These mechanisms remain pretty much untested and unused, eh? :)
bugc doesn't generate these... it only currently emits raw inline types and pointers rather than leveraging the format's provided resource lookup table. I guess solc also hasn't touched this yet.
For the conformance tests though, I suppose we do want to make sure that the machinery works here, even if the compilers do not use the machinery!
| } | ||
|
|
||
| const knownSourceIds = sourceIds(artifact); | ||
| if (knownSourceIds.size > 0) { |
There was a problem hiding this comment.
Can knownSourceIds ever be empty if referencedSourceIds isn't? If so, we probably don't want to silently skip this check.
| const interactive = await runSoldb({ | ||
| executable: soldb!, | ||
| args: [ | ||
| "trace", | ||
| tx.transactionHash, | ||
| "--rpc", | ||
| anvil.rpcUrl, | ||
| "--ethdebug-dir", | ||
| debugDir.spec, | ||
| "--interactive", | ||
| ], | ||
| stdin: "break Counter.sol:8\ncontinue\nq\n", | ||
| }); | ||
|
|
||
| expect(interactive.exitCode).toBe(0); | ||
| expect(interactive.stdout).toContain( | ||
| "Breakpoint set at Counter.sol:8, PC", | ||
| ); | ||
| expect(interactive.stdout).toContain("Breakpoint hit at step"); | ||
| expect(interactive.stdout).toContain("Counter.sol:8, PC"); |
There was a problem hiding this comment.
This stuff gets a bit brittle, no? No way to control soldb as a library for this?
| -DPEDANTIC=OFF \ | ||
| -DCMAKE_C_COMPILER_LAUNCHER=ccache \ | ||
| -DCMAKE_CXX_COMPILER_LAUNCHER=ccache | ||
| cmake --build solidity/build --target solc --parallel 2 |
There was a problem hiding this comment.
but I guess the price is worth of paying, right? :D
There was a problem hiding this comment.
yes, I think so. later, builds can be cached and performance can be improved :)
There was a problem hiding this comment.
https://github.com/argotorg/solc-bench#solc-bench-fetch-ref this might be interesting
3 participants
Add
conformancetests forethdebugproducers and consumers.