Skip to content

QA: deactivate tests requiring missing features#125

Merged
dannywillems merged 7 commits into
mainfrom
dw/deactivate-tests
Jun 11, 2026
Merged

QA: deactivate tests requiring missing features#125
dannywillems merged 7 commits into
mainfrom
dw/deactivate-tests

Conversation

@dannywillems

@dannywillems dannywillems commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

This has been cherry-picked from #104

@dannywillems dannywillems self-assigned this Jun 9, 2026
@dannywillems dannywillems linked an issue Jun 9, 2026 that may be closed by this pull request
@dannywillems dannywillems requested a review from nullcopy June 9, 2026 17:01
@nullcopy

nullcopy commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Hmm, idk about this one. Let's discuss at our next sync about deactivating all these

'wallet_unified_change.py', # needs z_shieldcoinbase funding step: zallet z_sendmany cannot spend mined coinbase (have 0)
'wallet_z_sendmany.py', # no zallet equiv yet: z_exportviewingkey
'wallet_z_shieldcoinbase.py', # investigate
'wallet_z_shieldcoinbase_multi_taddr.py', # investigate

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even though I thought it would be fixed by #98, it is still failing locally.

@oxarbitrage oxarbitrage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving the DISABLED_SCRIPTS mechanism.

The red CI here is a separate, pre-existing issue, not this PR: zebra's ZebraArgs.activation_heights defaults to {} while zallet activates NU5 at height 1, so create_cache.py crashes with "Missing Orchard tree state" before any test runs. Fixed in #127; rebasing this on top should go green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dannywillems

Copy link
Copy Markdown
Contributor Author

Approving the DISABLED_SCRIPTS mechanism.

The red CI here is a separate, pre-existing issue, not this PR: zebra's ZebraArgs.activation_heights defaults to {} while zallet activates NU5 at height 1, so create_cache.py crashes with "Missing Orchard tree state" before any test runs. Fixed in #127; rebasing this on top should go green.

Cherry-picking your commit for the contributor history. I was initially planning to take the ones from #104.

@dannywillems

dannywillems commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Approving the DISABLED_SCRIPTS mechanism.
The red CI here is a separate, pre-existing issue, not this PR: zebra's ZebraArgs.activation_heights defaults to {} while zallet activates NU5 at height 1, so create_cache.py crashes with "Missing Orchard tree state" before any test runs. Fixed in #127; rebasing this on top should go green.

Cherry-picking your commit for the contributor history. I was initially planning to take the ones from #104.

I confirm that it works locally with the following code (saved in test.py at the root of integration-tests).

import importlib
import os
import subprocess
import sys

sys.path.append('qa/pull-tester')
rpc_tests = importlib.import_module('rpc-tests')

src_dir = os.environ["SRC_DIR"]
build_dir = '.'
exeext = ''

class MyTestHandler(rpc_tests.RPCTestHandler):
    def start_test(self, args, stdout, stderr):
        return subprocess.Popen(
            args,
            universal_newlines=True,
            stdout=stdout,
            stderr=stderr)

test_list = [
  "wallet.py",
  "getmininginfo.py"
]
all_passed = rpc_tests.run_tests(MyTestHandler, test_list, src_dir, build_dir, exeext, jobs=len(test_list))
if all_passed == False:
    sys.exit(1)

and running with:

SRC_DIR=./ uv run test.py

Waiting for the CI to pass.

@dannywillems

Copy link
Copy Markdown
Contributor Author

Fixing the test by changing the logic of the default value.

dannywillems added a commit that referenced this pull request Jun 10, 2026
Before commit 153e0fd, NU5 wasn't activated,
and the test has been written with this setup in mind.

Without this fix, the test is failing. Instead of investigating if the behavior
is expected, which is outside the scope of
#125, reverting the behavior.
@dannywillems

Copy link
Copy Markdown
Contributor Author

For 26a5b77, run

uv run qa/rpc-tests/getrawtransaction_sidechain.py

Before the commit, the test should fail.

For 1917b83, run

uv run qa/rpc-tests/indexer.py

Same intent as the other test.

And default to {"NU5": 1}.
A test can set another value by overwriting the attribute.
Before commit 153e0fd, NU5 wasn't activated,
and the test has been written with this setup in mind.

Without this fix, the test is failing. Instead of investigating if the behavior
is expected, which is outside the scope of
#125, reverting the behavior.
This makes the test passing, after commit 153e0fd.
@dannywillems

Copy link
Copy Markdown
Contributor Author

CI is green. Squashing the fixup.

NEW_SCRIPTS = _without_disabled(NEW_SCRIPTS)
ZMQ_SCRIPTS = _without_disabled(ZMQ_SCRIPTS)

ALL_SCRIPTS = SERIAL_SCRIPTS + FLAKY_SCRIPTS + BASE_SCRIPTS + NEW_SCRIPTS + ZMQ_SCRIPTS + EXTENDED_SCRIPTS

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment says "ALL_SCRIPTS above is left complete so a disabled test can still be run explicitly by name" — but it isn't. ALL_SCRIPTS is assembled on this line, after _without_disabled() has already rebound SERIAL/FLAKY/BASE/NEW/ZMQ_SCRIPTS to their filtered versions, so the disabled tests aren't in it. And main() resolves explicitly-named tests against ALL_SCRIPTS, so naming a disabled test by hand selects nothing.

Verified locally on this branch (467d6c3c4):

$ ./qa/pull-tester/rpc-tests.py orchard_reorg.py
Running individually selected tests:
No valid test scripts specified. Check that your test is in one of the test lists in rpc-tests.py, ...
$ echo $?
0

orchard_reorg.py is a real test (and is in BASE_SCRIPTS), but it no-ops and exits 0 — so it reads as a pass while running nothing. On main the same command runs the test. This specifically affects the # pre-NU5 nuparams: migrate to ZebraArgs activation_heights group — the tests most likely to be re-run by name to check this PR's alignment re-enables them.

Fix is a one-line move — build ALL_SCRIPTS from the full lists before the filter block:

ALL_SCRIPTS = SERIAL_SCRIPTS + FLAKY_SCRIPTS + BASE_SCRIPTS + NEW_SCRIPTS + ZMQ_SCRIPTS + EXTENDED_SCRIPTS  # complete: menu for explicit-by-name

_DISABLED_FILES = {s.split()[0] for s in DISABLED_SCRIPTS}
def _without_disabled(scripts): ...
SERIAL_SCRIPTS = _without_disabled(SERIAL_SCRIPTS)   # run-set stays filtered
# ...

The default CI run is unaffected — it reads the filtered SERIAL_SCRIPTS + … + NEW_SCRIPTS names directly (line ~275), not ALL_SCRIPTS. (Alternatively, if disabled-means-unrunnable is intended, just drop the misleading comment.)

@dannywillems dannywillems merged commit d4690c7 into main Jun 11, 2026
35 checks passed
@dannywillems

Copy link
Copy Markdown
Contributor Author

Merging, and we will run agents with the following goals, for each test:

  • enable the test
  • run it
  • if fails -> open an issue with the error.
  • if succeeds -> open a patch removing it from the disabled list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QA: deactivate tests requiring missing features

3 participants