Two of the guards could not fail on the defect they name.
The measured-file guard filtered registrations by the dotted prefix, which
discarded exactly the bare-stem duplicate its own comment called out as the
threat, so it stayed green with the shadow copy live. It now counts every
registration shape-agnostically, and a second guard rejects any measured file
registered under a bare stem. Both walks were also one level too high: they
scanned `ReforceXY/` and so matched the out-of-scope reward-space modules.
The shuffle guard asserted the identity of the comparator the harness had
just installed, which a constant comparator would satisfy. It now asserts the
ORDER the loader produces — alphabetical exactly when no seed is set, and not
alphabetical when one is — with the seeded comparator rebuilt rather than read
out of the module, so the seed is actually exercised.
Also from review: the hyperopt eval-frequency test was a fixed point that
certified a reduction it never performed, so deleting the reduction left it
green; the net-arch tests asserted a key set and a type where the names
claimed widths; the numpy restore compared one element of a five-tuple and
could not see an advanced stream position; the CI step was named for an
ordering it cannot permute, since unittest sorts module and class order
alphabetically and consults no hook for either; and the sampler-pin comment
claimed a guarantee two wholesale call sites void.
92 tests green under four orderings; gate exits 0 at 57.1%; ruff clean;
basedpyright snapshot unchanged.
--entrypoint sh \
"${{ matrix.image }}" \
/workspace/scripts/run-coverage.sh -s "${{ matrix.context }}/tests" -v
- - name: Run runtime regressions under three orderings
+ - name: Run runtime regressions under three method orderings
if: matrix.context == 'ReforceXY'
run: |
docker run --rm \
(`:2244-2245`), since `_Samplers` has exactly two members and `:2218` raises
for anything else.
+CI additionally runs the ReforceXY suite three times with
+`FREQAI_QA_SHUFFLE_SEED=1|2|3`. The seed reorders test METHODS within each
+class — `unittest` sorts both module and class order alphabetically and
+consults no hook for either, so cross-module ordering is not permuted. A leaked
+global is still caught: `QaTestCase` restores the process globals in `setUp`
+and again in `addCleanup`, so every test starts from the import-time state
+whatever ran before it.
+
`fail_under` is an absolute floor, never per-module. To change it, re-measure
with the shipped configuration already in place — a measurement taken without
`branch` and `include_namespace_packages` reports a different denominator and
"add_state_info": False,
},
# `create_sampler` resolves `auto` through optunahub.load_module, which is an
- # outbound fetch to hub.optuna.org, so the sampler is pinned for every test. The
- # pin is only half the guarantee: `create_sampler` also defaults to "tpe" and
- # raises for anything outside `_Samplers`, so no config value reaches the network.
- # There is no `pruner` key: `create_pruner` is a staticmethod taking three ints.
+ # outbound fetch to hub.optuna.org, so the sampler is pinned here. The pin holds
+ # only while callers go through `**overrides`: two tests assign `rl_config_optuna`
+ # wholesale and would discard it. The guarantee that survives that is in the
+ # production default — `create_sampler` falls back to "tpe" and raises for anything
+ # outside `_Samplers`, so no config value reaches the network either way. There is
+ # no `pruner` key: `create_pruner` is a staticmethod taking three ints.
"rl_config_optuna": {"enabled": False, "sampler": "tpe"},
},
}
self.assertEqual(0.0, schedule(-0.5))
def test_it_reports_the_value_it_was_built_from(self):
- self.assertIn("0.5", repr(SimpleLinearSchedule(0.5)))
+ self.assertEqual("SimpleLinearSchedule(initial_value=0.5)", repr(SimpleLinearSchedule(0.5)))
class ScheduleClassificationTest(QaTestCase):
self.assertTrue(math.isnan(final))
def test_a_linear_request_builds_the_linear_schedule(self):
- self.assertIsInstance(get_schedule(LINEAR, 0.1), SimpleLinearSchedule)
+ schedule = get_schedule(LINEAR, 0.1)
+ self.assertIsInstance(schedule, SimpleLinearSchedule)
+ self.assertEqual(0.1, schedule(1.0))
+ self.assertEqual(0.0, schedule(0.0))
def test_every_other_request_builds_a_constant_schedule(self):
for kind in (CONSTANT, UNKNOWN):
class ArchitectureTest(QaTestCase):
- def test_the_on_policy_family_gets_a_separate_actor_and_critic_width(self):
+ def test_the_on_policy_family_returns_an_entry_per_network_and_shares_one_width(self):
+ # Production gives pi and vf the SAME width at every size, so the invariant is that
+ # they agree, not that they differ; a test named for a separation that does not
+ # exist would pass on the wrong widths.
for size in ReforceXY._NET_ARCH_SIZES:
with self.subTest(size=size):
arch = get_net_arch("MaskablePPO", size)
self.assertEqual({"pi", "vf"}, set(arch))
+ self.assertEqual(arch["pi"], arch["vf"])
+ self.assertEqual([128, 128], arch["pi"]) if size == "small" else None
def test_the_value_based_family_gets_a_shared_width_list(self):
for model_type in ("DQN", "QR-DQN"):
with self.subTest(model_type=model_type):
- self.assertIsInstance(get_net_arch(model_type, "small"), list)
+ self.assertEqual([128, 128], get_net_arch(model_type, "small"))
def test_a_grown_size_is_wider_than_a_small_one(self):
self.assertGreater(
def test_the_value_based_family_uses_the_evaluation_step_budget(self):
self.assertEqual(16, self._model(model_type="DQN", n_eval_steps=16).get_eval_freq(16_000))
- def test_hyperopt_reduces_the_interval_but_never_below_one(self):
+ def test_hyperopt_reduces_the_interval_by_the_configured_factor(self):
+ model = self._model(model_type="DQN", n_eval_steps=16)
+ self.assertEqual(16, model.get_eval_freq(16_000))
self.assertEqual(
- 1, self._model(model_type="DQN", n_eval_steps=1).get_eval_freq(16_000, hyperopt=True)
+ round(16 / ReforceXY._HYPEROPT_EVAL_FREQ_REDUCTION_FACTOR),
+ model.get_eval_freq(16_000, hyperopt=True),
)
+ def test_the_reduced_interval_never_falls_below_one(self):
+ model = self._model(model_type="DQN", n_eval_steps=1)
+ self.assertEqual(1, model.get_eval_freq(16_000, hyperopt=True))
+
if __name__ == "__main__":
unittest.main()
"""Guards the import identity and coverage denominator of the measured source.; requires the Freqtrade QA image."""
+import hashlib
import importlib
+import os
import random
import sys
import unittest
+from functools import cmp_to_key
from pathlib import Path
import numpy as np
importlib.import_module(_name)
+def _seeded_cmp(seed: str):
+ """The comparator qa_support builds for a given seed, rebuilt without touching _SEED."""
+
+ def compare(a: str, b: str) -> int:
+ ka = int.from_bytes(hashlib.blake2b(f"{seed}:{a}".encode(), digest_size=8).digest())
+ kb = int.from_bytes(hashlib.blake2b(f"{seed}:{b}".encode(), digest_size=8).digest())
+ return (ka > kb) - (ka < kb)
+
+ return compare
+
+
def _source_files_under(root: Path) -> list[tuple[str, Path]]:
"""Registered modules whose `__file__` is a `.py` under `root`, as (name, resolved path)."""
found = []
class SuiteContractTest(unittest.TestCase):
- def test_each_measured_source_file_is_registered_under_exactly_one_dotted_name(self):
- registered = [
- path
- for name, path in _source_files_under(MEASURED_TREE.parent)
- if name.startswith("ReforceXY.user_data")
- ]
+ def test_each_measured_source_file_is_registered_under_exactly_one_name(self):
+ # Shape-agnostic on purpose: filtering by the dotted prefix would discard the
+ # bare-stem registration this very comment calls out as the threat, and the guard
+ # would stay green with the shadow copy live.
+ registered = [path for _, path in _source_files_under(MEASURED_TREE)]
for src in _SOURCES:
with self.subTest(module=src.name):
matches = [path for path in registered if path == src.resolve()]
self.assertEqual(
1,
len(matches),
- "a measured file under exactly one dotted name; a second registration is a "
+ "a measured file under exactly one module name; a second registration is a "
"shadow copy carrying its own module state",
)
+ def test_no_measured_file_is_registered_under_a_bare_stem(self):
+ # CI puts `user_data/strategies` on PYTHONPATH, so a bare `import RLAgentStrategy`
+ # would resolve. A dotted name is the only one the production import uses.
+ for name, _ in _source_files_under(MEASURED_TREE):
+ with self.subTest(module=name):
+ self.assertTrue(
+ name.startswith("ReforceXY.user_data."),
+ "a measured file registered under a bare stem has a second module object, "
+ "and every process-global restore would clean the copy under test never uses",
+ )
+
def test_every_test_module_is_registered_under_its_bare_stem(self):
# The half that catches a shadow copy of the test tree itself. A measured-tree walk
# cannot observe `ReforceXY/tests`, so a dotted `from ReforceXY.tests.qa_support
"process globals it restores leak between tests",
)
- def test_the_shuffle_is_installed_on_the_loader(self):
+ def test_the_order_is_alphabetical_exactly_when_no_seed_is_set(self):
# `sortTestMethodsUsing` is a staticmethod on the LOADER; a TestCase has no such
# hook, and a plain function would bind as a method and raise TypeError at discovery.
+ # What is asserted here is the ORDER the loader produces, not the identity of the
+ # function it holds — a constant comparator installed by the same line would satisfy
+ # an identity check and defeat the shuffle.
+ names = ["test_c", "test_a", "test_b", "test_d"]
+ ordered = sorted(names, key=cmp_to_key(_shuffle_cmp))
+ if os.environ.get("FREQAI_QA_SHUFFLE_SEED", ""):
+ # A seeded run must NOT be alphabetical; the other guard in this class
+ # proves it is deterministic for a given seed.
+ self.assertNotEqual(sorted(names), ordered)
+ else:
+ self.assertEqual(sorted(names), ordered)
self.assertIs(_shuffle_cmp, unittest.TestLoader.sortTestMethodsUsing)
+ def test_a_seeded_order_differs_from_alphabetical(self):
+ names = ["test_c", "test_a", "test_b", "test_d"]
+ alphabetical = sorted(names)
+ for seed in ("1", "2", "3"):
+ with self.subTest(seed=seed):
+ shuffled = sorted(names, key=cmp_to_key(_seeded_cmp(seed)))
+ self.assertNotEqual(
+ alphabetical,
+ shuffled,
+ f"seed {seed} left the method order alphabetical, so the shuffle is inert",
+ )
+
def test_the_restore_puts_the_process_globals_back(self):
cache = _PRODUCTION.ReforceXY._action_masks_cache
# Dirty every global the way a test that trains a model would.
self.assertEqual(
_NUMPY_STATE_AT_IMPORT[1].tolist(),
np.random.get_state()[1].tolist(),
+ "the numpy bit generator was not rewound",
+ )
+ self.assertEqual(
+ int(_NUMPY_STATE_AT_IMPORT[2]),
+ int(np.random.get_state()[2]),
+ "the numpy stream position advanced and was not restored",
)
if _TORCH_STATE_AT_IMPORT is not None:
self.assertTrue(torch.equal(torch.random.get_rng_state(), _TORCH_STATE_AT_IMPORT))
repositories / freqai-strategies.git / commitdiff
