test: Convert the ethdebug schema conformity tests to unittest by djolertrk · Pull Request #17055 · argotorg/solidity

djolertrk · 2026-09-30T12:47:25Z

Split out of #16990 as requested. The suite was the only pytest user in the repo. This converts it as it is on develop to unittest, runs it as a script with --solc-binary-path, removes conftest.py and drops pytest from CI. No new checks.

cameel

I think we should keep conftest.py, just convert it into a unittest runner script. This would allow us to structure it better and hopefully also make the diff less intimidating.

cameel · 2026-09-30T18:43:09Z

Also, related issue: #16506.

clonker

You can no longer pick individual tests. The parametrized cases became subTest loops, so 10 tests became 8 and single cases can't be selected. The runner also doesn't forward any unittest arguments. python -m unittest refuses to run because of the config assert.
I also think that #16506 gets harder. That issue is about running the schema checks on many inputs. In pytest that's a glob in params=, and each input gets its own test ID. In unittest it means a loop inside every method, or generating test classes through the load_tests protocol.

So yeah, idk, I am camp pytest. If my arguments do not make sense to you @cameel then perhaps we just agree to disagree and you go with unittest. Just, again, I do not think it is the right move :)

clonker · 2026-10-01T19:17:56Z

-        assert [instruction["offset"] for instruction in instructions] == sorted(
-            instruction["offset"] for instruction in instructions
+    def setUp(self):
+        assert self.config is not None, "Run the tests through test/ethdebugSchemaTests.py."


if this fails it is reported as test failure; in unittest any AssertionError counts as failure. Only other exceptions count as errors.

in pytest this is actually differentiated by location: in the test, it's a failure, in the fixture it's an error

Fair point. But honestly, what matters the most to me is being able to distinguish assumptions from tests when I read the code. So even if they raise the same error in the end it's useful.

It can be solved though. To get the most out of it we should also use the failureException property to make unittest use something else than AssertionError.

The real question is if pytest provides some alternative helpers for testing that we could use instead of abusing assert. AFAIK the only way to do that is basically to use unittest test cases with pytest. Which I'd be fine with, but then why even bother having pytest in the first place?

clonker · 2026-10-01T19:19:43Z

-        assert len(instructions) > 0
-        assert [instruction["offset"] for instruction in instructions] == sorted(
-            instruction["offset"] for instruction in instructions
+    def setUp(self):


every single test will execute this, maybe setUpClass can help here, but i am not sure how that plays together with the self.config, perhaps that can be injected into the class? but yeah. hacky. or inefficient. do you have any ideas?

The core idea of pytest-like fixtures is fine. But we don't really need pytest for that. We get almost the same thing if we simply put a @functools.cache decorator on them. What we're missing is this implicit system where fixtures are automatically matched with parameters based on names and automatically become available everywhere without importing them. I would not be missing that really. It makes it harder to understand how things are connected.

cameel

Well, this is not yet blocking the ethdebug PR (we still have a bit to go there) so we can take our time here to make sure everyone is on board with the change here. TBH I expected this change to be less controversial given no reaction earlier to my complaints about pytest.

I feel pretty strongly about this in general, and I'd definitely not go with pytest in a big Python project, but this is not the case here. It's just a small bit of our testing infrastructure so no big deal if in the end we go with an inferior solution. I'd still prefer to be convinced that it actually has merit here. As far as I can tell for now, we can still achieve what we need with unittest without the downsides of pytest.

cameel · 2026-10-02T02:33:37Z

-        assert [instruction["offset"] for instruction in instructions] == sorted(
-            instruction["offset"] for instruction in instructions
+    def setUp(self):
+        assert self.config is not None, "Run the tests through test/ethdebugSchemaTests.py."


Fair point. But honestly, what matters the most to me is being able to distinguish assumptions from tests when I read the code. So even if they raise the same error in the end it's useful.

It can be solved though. To get the most out of it we should also use the failureException property to make unittest use something else than AssertionError.

The real question is if pytest provides some alternative helpers for testing that we could use instead of abusing assert. AFAIK the only way to do that is basically to use unittest test cases with pytest. Which I'd be fine with, but then why even bother having pytest in the first place?

cameel · 2026-10-02T03:08:05Z

-        assert len(instructions) > 0
-        assert [instruction["offset"] for instruction in instructions] == sorted(
-            instruction["offset"] for instruction in instructions
+    def setUp(self):


The core idea of pytest-like fixtures is fine. But we don't really need pytest for that. We get almost the same thing if we simply put a @functools.cache decorator on them. What we're missing is this implicit system where fixtures are automatically matched with parameters based on names and automatically become available everywhere without importing them. I would not be missing that really. It makes it harder to understand how things are connected.

cameel · 2026-10-02T04:47:06Z

You can no longer pick individual tests. The parametrized cases became subTest loops, so 10 tests became 8 and single cases can't be selected.

I find subTest() acceptable most of the time, because typical test suites have a lot of tests, few parameters and are pretty fast. What we have here is a bit of a special case, because it's all basically one huge test with a huge parameter set, with each individual subtest being quite heavy (runs the whole compiler). You're right that for this the ability to rerun a specific combination is a bit more important. Still, you can have parametrization in unittest through parameterized without getting the other downsides of pytest.

The runner also doesn't forward any unittest arguments. python -m unittest refuses to run because of the config assert.

Yes, that's definitely a pain. unittest does not have a great story around it. The config injection is a bit awkward.

I also think that #16506 gets harder. That issue is about running the schema checks on many inputs. In pytest that's a glob in params=, and each input gets its own test ID. In unittest it means a loop inside every method, or generating test classes through the load_tests protocol.

You can use glob() with unittest too. So this really boils down to parametrization again.

So yeah, idk, I am camp pytest. If my arguments do not make sense to you @cameel then perhaps we just agree to disagree and you go with unittest. Just, again, I do not think it is the right move :)

I'm open to be convinced by good arguments so let's talk it through.

pytest has some nice things but the way it achieves them is not simple. It goes too far in trying to make things easy by just hiding the connections and that has a cost in underlying complexity. I like the straightforwardness of unittest and I think it's underappreciated how far you can go with it. A lot of what pytest does is either magic that we're better off without or things that are doable with unittest as well.

This is how the trade-off looks to me:

Good:

Parametrization (doable in unittest via subTest() or parameterized)

Fixtures (doable in unittest via @cache)

Ability to define command-line parameters (very clunky but doable)

Bad:

Breaks distinction between assertions and test checks.

Hidden complexity. It has to rewrite the AST of your code to inject its support machinery. Fine as long as it works but when it breaks it may be hard to understand what is happening.

Magic fixture parameter injection.

Magic fixture discovery without explicit imports.

Just to not leave this open after you spend some time thinking about it and writing it up: Aside from the additional infrastructure we maintain ourselves (custom runner, config convention, sys.path setup in the runner, caching, potentially argument forwarding, parameterized as additional dependency), I think extensibility of the test suite is what matters most and for pytest it's a three line change to one fixture.
I don't perceive the magic/hidden complexity as bad, it's very well tested and proven to work in many (also large) projects.
In the end I think it's a bit a matter of taste and perhaps philosophy. I'd rather not hold up or even block any efforts and since the rewrite has already happened we might as well just go with it.

djolertrk · 2026-10-02T13:14:40Z

Thanks @clonker, fair points. I pushed a commit that addresses them within unittest (@cameel please also check), and it covers following:

the program checks are split per bytecode again, so it's 10 tests and each can be run alone,
the runner takes -k like python -m unittest -k, e.g. test/ethdebugSchemaTests.py --solc-binary-path build/solc/solc -k test_deployed_program_schema
the compiler runs once and the output is shared via @cache,
a missing config is now reported as an error, not a failure.

python -m unittest still can't run the tests on its own, since the solc path has to come from somewhere. For #16506 the inputs can be globbed into one test class per file. But, the pytest vs unittest call is up to you two, I really do not have a strong opinion here.

djolertrk · 2026-10-12T14:38:37Z

any comment on this? can we merge it? @cameel @clonker

clonker

I am fine with it but I am leaving it up to @cameel to approve this one

djolertrk · 2026-10-19T06:57:50Z

cc @cameel

cameel

I don't see any big problems here, everything was addressed, but it could use some tweaks/cleanup of smaller stuff before we merge.

The helpers locating the schemas and building the schema registry move unchanged into schema_helpers.py; conftest.py keeps thin fixtures that call them. This separates them from pytest ahead of converting the tests to unittest.

The suite was the only pytest user in the repository, and pytest turns `assert` into its testing tool, so a broken assumption of a test reads the same as a failure of the code under test. The tests are now methods of a unittest TestCase using its assertions, and check the same things as before. test/ethdebugSchemaTests.py replaces conftest.py, following test/pyscriptTests.py: it discovers every test module in test/ethdebugSchemaTests/, so a new one is not silently skipped, and gives each test case a configuration with the compiler path instead of reading it from a global.

Check the creation and the deployed programs in tests of their own again, as the pytest version did, and let the runner pass `-k` patterns to the test loader so that a single test can be run. Unlike `-k`, the loader matches a pattern without wildcards against the whole test name, so the runner wraps such a pattern in wildcards. Every test reads the same compiler output, so the input is loaded and compiled once in setUpClass, and the runner sets the configuration on the test case classes. A missing configuration is an error of the setup, not a failed check. Both test runners check the result with wasSuccessful().

cameel

This seems fine now.

github-actions Bot added the external contribution ⭐ label Sep 30, 2026

djolertrk mentioned this pull request Sep 30, 2026

ethdebug: Emit type documents and state variable pointer templates in resources #16990

Open

cameel requested changes Sep 30, 2026

View reviewed changes

Comment thread test/ethdebugSchemaTests/conftest.py

Comment thread test/ethdebugSchemaTests/conftest.py

Comment thread .circleci/config.yml Outdated

Comment thread test/ethdebugSchemaTests/test_ethdebug_schema_conformity.py Outdated

cameel added testing 🔨 refactor ethdebug and removed refactor labels Sep 30, 2026

djolertrk force-pushed the feature/ethdebug-schema-tests-unittest branch from 3b875c4 to 029b668 Compare October 1, 2026 12:06

clonker reviewed Oct 1, 2026

View reviewed changes

cameel reviewed Oct 2, 2026

View reviewed changes

djolertrk requested review from cameel and clonker October 5, 2026 08:23

clonker reviewed Oct 12, 2026

View reviewed changes

cameel reviewed Oct 19, 2026

View reviewed changes

djolertrk added 3 commits October 19, 2026 17:12

test: Move the ethdebug schema helpers out of conftest.py

292ba39

The helpers locating the schemas and building the schema registry move unchanged into schema_helpers.py; conftest.py keeps thin fixtures that call them. This separates them from pytest ahead of converting the tests to unittest.

djolertrk force-pushed the feature/ethdebug-schema-tests-unittest branch from 2779545 to d18fd9c Compare October 19, 2026 15:17

djolertrk requested a review from cameel October 19, 2026 15:19

cameel approved these changes Oct 21, 2026

View reviewed changes

cameel merged commit e608896 into argotorg:develop Oct 21, 2026
83 checks passed

Conversation

djolertrk commented Sep 30, 2026

Uh oh!

cameel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameel commented Sep 30, 2026

Uh oh!

clonker left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clonker Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clonker Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

cameel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

cameel Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clonker Oct 2, 2026

Choose a reason for hiding this comment

Uh oh!

djolertrk commented Oct 2, 2026

Uh oh!

djolertrk commented Oct 12, 2026

Uh oh!

clonker left a comment

Choose a reason for hiding this comment

Uh oh!

djolertrk commented Oct 19, 2026

Uh oh!

cameel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

clonker left a comment •

edited

Loading

clonker Oct 1, 2026 •

edited

Loading

cameel Oct 2, 2026 •

edited

Loading

clonker Oct 1, 2026 •

edited

Loading

cameel left a comment •

edited

Loading

cameel Oct 2, 2026 •

edited

Loading

cameel Oct 2, 2026 •

edited

Loading