Skip to content

Commit f1af025

Browse files
committed
feat: allow tests-less .module archives; re-render if conformance needs tests
A module rendered without a conformance-tests script has no tests/ folder, so its .module archive has none either. Requiring tests/ wrongly rejected such archives. - Archive validation now requires only code/ (with .git); tests/ is optional and validated only when present. - Add PlainModule.archive_has_conformance_tests() (peeks the zip for tests/). - Add partial_rendering.archive_missing_conformance_tests(): an archive-only module lacking tests/ while conformance is enabled cannot be consumed as-is. It surfaces a new 'missing_conformance_tests' render-state reason (a hard blocker taking precedence over spec/code changes) so the user is warned and offered quit / rerender-that-module. - On rerender the module rebuilds (code + tests) and its .module is removed; on quit the archive is left untouched. The eager materialize skips such archives so they route to a re-render. - Tests for code-only archive validate/materialize/unpack and for the detection + choice.
1 parent 84c5406 commit f1af025

6 files changed

Lines changed: 180 additions & 39 deletions

‎partial_rendering.py‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class PlainModuleRenderState:
1212
last_render_module: PlainModule
1313
last_render_frid: str | None
1414
change: PlainModule | None = None
15-
change_type: Literal["spec_change", "code_change"] | None = None
15+
change_type: Literal["spec_change", "code_change", "missing_conformance_tests"] | None = None
1616

1717

1818
@dataclass
@@ -56,6 +56,19 @@ def code_change(plain_module: PlainModule) -> PlainModule | None:
5656
return None
5757

5858

59+
def archive_missing_conformance_tests(plain_module: PlainModule, render_conformance_tests: bool) -> PlainModule | None:
60+
"""Return the first module available only as a "<module>.module" archive that has no conformance
61+
tests while conformance testing is enabled. Such an archive cannot be consumed as-is (regression
62+
needs the tests), so the module must be re-rendered."""
63+
if not render_conformance_tests:
64+
return None
65+
all_modules = plain_module.all_required_modules + [plain_module]
66+
for _module in all_modules:
67+
if _module.is_archived_only() and not _module.archive_has_conformance_tests():
68+
return _module
69+
return None
70+
71+
5972
def module_comes_before_or_equal(
6073
all_required_modules: list[PlainModule],
6174
module1: PlainModule,
@@ -70,15 +83,24 @@ def module_comes_before_or_equal(
7083
raise ValueError(f"Module {module1.module_name} and {module2.module_name} not found in {all_required_modules}")
7184

7285

73-
def get_plain_module_render_state(plain_module: PlainModule) -> PlainModuleRenderState | None:
86+
def get_plain_module_render_state(
87+
plain_module: PlainModule, render_conformance_tests: bool = False
88+
) -> PlainModuleRenderState | None:
7489
sc = spec_change(plain_module)
7590
cc = code_change(plain_module)
91+
mt = archive_missing_conformance_tests(plain_module, render_conformance_tests)
7692
all_required_modules = plain_module.all_required_modules
7793
last_rendered_module_name, last_rendered_frid = plain_module.get_module_render_status()
78-
if last_rendered_module_name is None and last_rendered_frid is None:
94+
if last_rendered_module_name is None and last_rendered_frid is None and mt is None:
7995
return None
8096

81-
if last_rendered_module_name == plain_module.module_name:
97+
if last_rendered_module_name is None:
98+
# Nothing has been rendered yet, but an archived module blocks consumption (no tests).
99+
# Anchor the state on that module so the user is prompted to re-render it.
100+
assert mt is not None # guaranteed by the early return above
101+
module = mt
102+
last_rendered_frid = None
103+
elif last_rendered_module_name == plain_module.module_name:
82104
module = plain_module
83105
else:
84106
found_module: PlainModule | None = None
@@ -99,6 +121,13 @@ def get_plain_module_render_state(plain_module: PlainModule) -> PlainModuleRende
99121
change_type=None,
100122
)
101123

124+
# An archive that lacks conformance tests (while testing is enabled) is a hard blocker: it cannot
125+
# be consumed as-is, so it takes precedence over spec/code changes.
126+
if mt is not None:
127+
pr.change = mt
128+
pr.change_type = "missing_conformance_tests"
129+
return pr
130+
102131
if sc is None and cc is None:
103132
return pr
104133

@@ -129,6 +158,8 @@ def get_all_affected_modules_from_change(
129158
start_module = plain_module.get_next_module(plain_module_render_state.change.module_name)
130159
else:
131160
start_module = plain_module_render_state.change
161+
elif plain_module_render_state.change_type == "missing_conformance_tests":
162+
start_module = plain_module_render_state.change
132163
else:
133164
raise ValueError(f"Unknown change type: {plain_module_render_state.change_type}")
134165

‎plain2code.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,11 @@ def render( # noqa: C901
216216

217217
# A built module can be distributed as a "<module>.module" zip archive instead of an unpacked directory.
218218
# Extract any such module to a scratch location up front. Scratch dirs are removed in main()'s finally.
219+
# Skip an archive-only module that lacks conformance tests while testing is enabled: it cannot be
220+
# consumed as-is, so leave it unmaterialized so it is detected below as needing a re-render.
219221
for module in plain_module.all_required_modules + [plain_module]:
222+
if args.render_conformance_tests and module.is_archived_only() and not module.archive_has_conformance_tests():
223+
continue
220224
module.materialize()
221225

222226
# The module_metadata file lives outside the code git repo. That means that a crash mid-render can leave it
@@ -227,7 +231,7 @@ def render( # noqa: C901
227231

228232
render_choice = None
229233
if render_range is None:
230-
plain_module_render_state = get_plain_module_render_state(plain_module)
234+
plain_module_render_state = get_plain_module_render_state(plain_module, args.render_conformance_tests)
231235
if plain_module_render_state is not None:
232236
render_choices = get_render_choices(plain_module, plain_module_render_state, args.force_render)
233237
ask_user = True

‎plain_modules.py‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -50,34 +50,44 @@ def get_module_tests_folder(modules_base_folder: str, module_name: str) -> str:
5050
return os.path.join(modules_base_folder, module_name, MODULE_TESTS_SUBFOLDER)
5151

5252

53-
def _validate_module_tree(root: str, archive_path: str) -> None:
54-
"""Verify an extracted module tree has valid code/ and tests/ git repositories."""
55-
for subfolder in (MODULE_CODE_SUBFOLDER, MODULE_TESTS_SUBFOLDER):
56-
repo_path = os.path.join(root, subfolder)
57-
if not os.path.isdir(repo_path):
58-
raise InvalidModuleArchiveError(
59-
f"Module archive '{archive_path}' is missing the '{subfolder}/' folder at its root. "
60-
"The archive must contain the module folder's contents (code/, tests/, ...) at its "
61-
"root, not nested under a top-level directory."
62-
)
63-
if not os.path.isdir(os.path.join(repo_path, ".git")):
53+
def _validate_module_repo(repo_path: str, subfolder: str, archive_path: str) -> None:
54+
"""Validate one extracted git repo (code/ or tests/) inside a module archive."""
55+
if not os.path.isdir(os.path.join(repo_path, ".git")):
56+
raise InvalidModuleArchiveError(
57+
f"Module archive '{archive_path}' has no git repository in '{subfolder}/' "
58+
"(the archive must include the '.git' directory)."
59+
)
60+
try:
61+
repo = Repo(repo_path)
62+
if repo.bare or repo.head.is_detached:
6463
raise InvalidModuleArchiveError(
65-
f"Module archive '{archive_path}' has no git repository in '{subfolder}/' "
66-
"(the archive must include the '.git' directory)."
64+
f"The git repository in '{subfolder}/' of module archive '{archive_path}' "
65+
"must be a working tree checked out on a branch."
6766
)
68-
try:
69-
repo = Repo(repo_path)
70-
if repo.bare or repo.head.is_detached:
71-
raise InvalidModuleArchiveError(
72-
f"The git repository in '{subfolder}/' of module archive '{archive_path}' "
73-
"must be a working tree checked out on a branch."
74-
)
75-
except InvalidModuleArchiveError:
76-
raise
77-
except Exception as e:
78-
raise InvalidModuleArchiveError(
79-
f"The git repository in '{subfolder}/' of module archive '{archive_path}' is invalid: {e}"
80-
) from e
67+
except InvalidModuleArchiveError:
68+
raise
69+
except Exception as e:
70+
raise InvalidModuleArchiveError(
71+
f"The git repository in '{subfolder}/' of module archive '{archive_path}' is invalid: {e}"
72+
) from e
73+
74+
75+
def _validate_module_tree(root: str, archive_path: str) -> None:
76+
"""Verify an extracted module tree. ``code/`` is required. ``tests/`` is optional: a module
77+
rendered without a conformance-tests script has no tests folder, so its archive has none either.
78+
When ``tests/`` is present it must be a valid git repo."""
79+
code_path = os.path.join(root, MODULE_CODE_SUBFOLDER)
80+
if not os.path.isdir(code_path):
81+
raise InvalidModuleArchiveError(
82+
f"Module archive '{archive_path}' is missing the '{MODULE_CODE_SUBFOLDER}/' folder at its root. "
83+
"The archive must contain the module folder's contents (code/, optionally tests/, ...) at its "
84+
"root, not nested under a top-level directory."
85+
)
86+
_validate_module_repo(code_path, MODULE_CODE_SUBFOLDER, archive_path)
87+
88+
tests_path = os.path.join(root, MODULE_TESTS_SUBFOLDER)
89+
if os.path.isdir(tests_path):
90+
_validate_module_repo(tests_path, MODULE_TESTS_SUBFOLDER, archive_path)
8191

8292

8393
def _extract_module_archive(archive_path: str, dest: str) -> None:
@@ -199,6 +209,15 @@ def has_module_archive(self) -> bool:
199209
def is_archived_only(self) -> bool:
200210
return not os.path.isdir(self._default_module_folder) and self.has_module_archive()
201211

212+
def archive_has_conformance_tests(self) -> bool:
213+
"""True if the "<module>.module" archive contains a tests/ folder. A module rendered without
214+
a conformance-tests script has no tests/, so its archive has none either."""
215+
if not self.has_module_archive() or not zipfile.is_zipfile(self.module_archive_path):
216+
return False
217+
prefix = MODULE_TESTS_SUBFOLDER + "/"
218+
with zipfile.ZipFile(self.module_archive_path) as zf:
219+
return any(name.startswith(prefix) for name in zf.namelist())
220+
202221
def materialize(self) -> None:
203222
"""Make an archive-only module readable without populating plain_modules/<module>/.
204223

‎tests/test_plain_modules.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,38 @@ def test_code_hash_is_location_independent(fixtures_dir):
554554
assert mod_a.get_module_code_hash() == mod_b.get_module_code_hash()
555555

556556

557+
def test_archive_has_conformance_tests(solo_module):
558+
_setup_full_module(solo_module) # code/ + tests/
559+
_archive_module(solo_module)
560+
assert solo_module.archive_has_conformance_tests() is True
561+
562+
563+
def test_code_only_archive_materializes_without_tests(solo_module):
564+
"""A module rendered with conformance tests off has no tests/, so its archive has none. It must
565+
still validate and materialize (tests/ is optional)."""
566+
_init_repo_with_finished_frid(solo_module.module_build_folder, solo_module.module_name, "1")
567+
_write_metadata(solo_module, {"source_hash": "seed"})
568+
_archive_module(solo_module) # code/ + .codeplain, no tests/
569+
570+
assert solo_module.archive_has_conformance_tests() is False
571+
assert solo_module.is_archived_only()
572+
573+
solo_module.materialize() # must not raise
574+
assert os.path.isdir(os.path.join(solo_module.module_build_folder, ".git"))
575+
assert not os.path.isdir(solo_module.module_conformance_tests_folder)
576+
solo_module.cleanup_scratch()
577+
578+
579+
def test_ensure_module_unpacked_code_only_archive(solo_module):
580+
_init_repo_with_finished_frid(solo_module.module_build_folder, solo_module.module_name, "1")
581+
_archive_module(solo_module) # code/ only
582+
583+
solo_module.ensure_module_unpacked()
584+
assert os.path.isdir(os.path.join(solo_module._default_module_folder, "code", ".git"))
585+
assert not os.path.isdir(os.path.join(solo_module._default_module_folder, "tests"))
586+
assert not os.path.exists(solo_module.module_archive_path)
587+
588+
557589
def test_materialize_rejects_non_zip(solo_module):
558590
with open(solo_module.module_archive_path, "w") as f:
559591
f.write("not a zip")

‎tests/test_prepare_repositories_layout.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import pytest
2020

2121
from git_utils import FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE, add_all_files_and_commit, init_git_repo
22+
from partial_rendering import archive_missing_conformance_tests, get_plain_module_render_state, get_render_choices
2223
from plain_modules import PlainModule
2324
from render_machine.actions.prepare_repositories import PrepareRepositories
2425
from render_machine.conformance_tests import CONFORMANCE_TESTS_DEFINITION_FILE_NAME, ConformanceTests
@@ -183,6 +184,43 @@ def test_full_render_clones_from_archived_required_module(root_module):
183184
previous.cleanup_scratch()
184185

185186

187+
def test_archive_missing_conformance_tests_detects_code_only_required_module(root_module):
188+
"""A required module shipped as a code-only .module (no tests/) is flagged only when conformance
189+
testing is enabled."""
190+
previous = root_module.required_modules[-1]
191+
_init_repo_with_finished_frid(previous.module_build_folder, previous.module_name)
192+
_archive_module(previous) # code/ only, no tests/
193+
194+
assert archive_missing_conformance_tests(root_module, True) is previous
195+
assert archive_missing_conformance_tests(root_module, False) is None
196+
197+
198+
def test_archive_with_tests_not_flagged(root_module):
199+
previous = root_module.required_modules[-1]
200+
_init_repo_with_finished_frid(previous.module_build_folder, previous.module_name)
201+
_init_repo_with_finished_frid(previous.module_conformance_tests_folder, previous.module_name)
202+
_archive_module(previous) # code/ + tests/
203+
204+
assert archive_missing_conformance_tests(root_module, True) is None
205+
206+
207+
def test_render_state_flags_missing_tests_and_offers_rerender(root_module):
208+
"""A code-only archive (conformance on) surfaces a 'missing_conformance_tests' render state with
209+
a rerender choice and a quit choice."""
210+
previous = root_module.required_modules[-1]
211+
_init_repo_with_finished_frid(previous.module_build_folder, previous.module_name)
212+
_archive_module(previous) # code/ only
213+
214+
state = get_plain_module_render_state(root_module, render_conformance_tests=True)
215+
assert state is not None
216+
assert state.change_type == "missing_conformance_tests"
217+
assert state.change.module_name == previous.module_name
218+
219+
choice_types = {c.choice_type for c in get_render_choices(root_module, state).values()}
220+
assert "quit" in choice_types
221+
assert "rerender_affected" in choice_types
222+
223+
186224
def test_conformance_tests_resolver_overrides_default(tmp_build_folder):
187225
"""A resolver maps an archived required module's name to its resolved (scratch) tests folder;
188226
unknown names fall back to the default base-folder path."""

‎tui/plain_module_render_choice_tui.py‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,21 +118,38 @@ def on_mount(self) -> None:
118118
info_panel.mount(change_box)
119119

120120
if pr.change:
121-
title_start = "Spec changes" if pr.change_type == "spec_change" else "Code changes"
122121
is_required_module = pr.change.module_name != self.plain_module.module_name
123-
change_box.mount(
124-
Label(
125-
f"--- {title_start} detected in {'required ' if is_required_module else 'current '}module [#5593FF]{pr.change.module_name}[/] ---",
126-
classes="rendering-info-row",
122+
module_kind = "required " if is_required_module else "current "
123+
if pr.change_type == "missing_conformance_tests":
124+
change_box.mount(
125+
Label(
126+
f"--- Conformance tests missing from the archive of {module_kind}module "
127+
f"[#5593FF]{pr.change.module_name}[/] ---",
128+
classes="rendering-info-row",
129+
)
127130
)
128-
)
129-
if is_required_module:
130131
change_box.mount(
131132
Label(
132-
f"{title_start} in a required module may affect the current module",
133+
"The .module archive has no conformance tests but testing is enabled; "
134+
"the module must be re-rendered (its .module file will be removed).",
133135
classes="rendering-info-title",
134136
)
135137
)
138+
else:
139+
title_start = "Spec changes" if pr.change_type == "spec_change" else "Code changes"
140+
change_box.mount(
141+
Label(
142+
f"--- {title_start} detected in {module_kind}module [#5593FF]{pr.change.module_name}[/] ---",
143+
classes="rendering-info-row",
144+
)
145+
)
146+
if is_required_module:
147+
change_box.mount(
148+
Label(
149+
f"{title_start} in a required module may affect the current module",
150+
classes="rendering-info-title",
151+
)
152+
)
136153

137154
elif pr.last_render_module.is_module_fully_rendered():
138155
change_box.mount(Label("The current module is fully rendered.", classes="rendering-info-title"))

0 commit comments

Comments
 (0)