diff --git a/.gitignore b/.gitignore index 7893c1e3..3ea3be64 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,5 @@ dist .coverage logging_config.yaml + +.claude/worktrees/ \ No newline at end of file diff --git a/codeplain_REST_api.py b/codeplain_REST_api.py index 3747d544..9f3ccc8f 100644 --- a/codeplain_REST_api.py +++ b/codeplain_REST_api.py @@ -266,6 +266,9 @@ def render_functional_requirement( required_modules: dict, include_unittests: bool, run_state: RunState, + is_regeneration: bool = False, + old_functional_requirement_text: Optional[str] = None, + code_functional_requirements: Optional[list] = None, ) -> dict[str, str]: """ Renders the content of a functionality based on the provided ID, @@ -305,8 +308,15 @@ def render_functional_requirement( "module_name": module_name, "required_modules": required_modules, "include_unittests": include_unittests, + "is_regeneration": is_regeneration, } + if old_functional_requirement_text is not None: + payload["old_functional_requirement_text"] = old_functional_requirement_text + + if code_functional_requirements is not None: + payload["code_functional_requirements"] = code_functional_requirements + return self.post_request(endpoint_url, headers, payload, run_state) def distill_conformance_test_memory( @@ -365,6 +375,7 @@ def render_conformance_tests( conformance_tests_folder_name, conformance_tests_json, run_state: RunState, + is_regeneration: bool = False, ): endpoint_url = f"{self.api_url}/render_conformance_tests" headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"} @@ -380,6 +391,7 @@ def render_conformance_tests( "required_modules": required_modules, "conformance_tests_folder_name": conformance_tests_folder_name, "conformance_tests_json": conformance_tests_json, + "is_regeneration": is_regeneration, } response = self.post_request(endpoint_url, headers, payload, run_state) diff --git a/git_utils.py b/git_utils.py index 53b5b331..1ac8bf79 100644 --- a/git_utils.py +++ b/git_utils.py @@ -17,6 +17,7 @@ # Following messages are used as checkpoints in the git history # Changing them will break backwards compatibility so change them with care FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE = "[Codeplain] functionality ID (FRID):{} fully implemented" +FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE = "[Codeplain] functionality ID (FRID):{} fully regenerated" INITIAL_COMMIT_MESSAGE = "[Codeplain] Initial module commit" BASE_FOLDER_COMMIT_MESSAGE = "[Codeplain] Initialize build with Base Folder content" @@ -325,6 +326,24 @@ def has_commit_for_frid(repo_path: Union[str, os.PathLike], frid: str, module_na return bool(_get_commit_with_frid(Repo(repo_path), frid, module_name)) +def get_frids_regenerated_after_frid(repo_path: Union[str, os.PathLike], frid: str, module_name: str) -> list[str]: + """The functionalities regenerated after the "fully implemented" commit of `frid`. + + A reset to that commit removes these regenerations. + """ + repo = Repo(repo_path) + commit = _get_commit_with_frid(repo, frid, module_name) + if not commit: + return [] + + escaped_module_message = MODULE_NAME_MESSAGE.format(module_name).replace("[", "\\[").replace("]", "\\]") + subjects = repo.git.log(f"{commit}..HEAD", "--grep", escaped_module_message, "--format=%s").splitlines() + regenerated_pattern = re.compile( + re.escape(FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE).replace(re.escape("{}"), "(.+)") + ) + return [match.group(1) for subject in subjects if (match := regenerated_pattern.fullmatch(subject))] + + def _get_base_folder_commit(repo: Repo) -> str: """Finds commit related to copy of the base folder.""" return _get_commit_with_message(repo, BASE_FOLDER_COMMIT_MESSAGE) @@ -430,6 +449,10 @@ def get_last_rendered_functionality(repo_path: Union[str, os.PathLike]) -> tuple return None, None repo = Repo(repo_path) + + # Only a "fully implemented" commit marks how far the module is rendered. A "fully regenerated" + # commit sits on top of that frontier, because a single functionality was rendered again in place. + # Treating it as the frontier would report the regenerated frid as the last rendered one. grep_pattern = FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE.format(".*") grep_pattern = grep_pattern.replace("[", "\\[").replace("]", "\\]") commit_sha = repo.git.rev_list(repo.active_branch.name, "--grep", grep_pattern, "-n", "1") diff --git a/module_renderer.py b/module_renderer.py index 20280fb6..da689b45 100644 --- a/module_renderer.py +++ b/module_renderer.py @@ -26,6 +26,7 @@ def __init__( event_bus: EventBus, stop_event: threading.Event | None = None, enter_pause_event: threading.Event | None = None, + is_regenerate: bool = False, ): self.codeplainAPI = codeplainAPI self.plain_module = plain_module @@ -36,12 +37,14 @@ def __init__( self.event_bus = event_bus self.stop_event = stop_event self.enter_pause_event = enter_pause_event + self.is_regenerate = is_regenerate def _build_render_context_for_module( self, plain_module: PlainModule, memory_manager: MemoryManager, render_range: list[str] | None, + is_regenerate: bool, ) -> RenderContext: return RenderContext( self.codeplainAPI, @@ -63,6 +66,7 @@ def _build_render_context_for_module( test_script_timeout=self.args.test_script_timeout, stop_event=self.stop_event, enter_pause_event=self.enter_pause_event, + is_regenerate=is_regenerate, ) def _render_module( @@ -83,7 +87,12 @@ def _render_module( render_range = self.render_choice.render_range if render_range is not None: + if self.is_regenerate: + plain_module.ensure_frid_can_be_regenerated(render_range[0]) plain_module.ensure_previous_frid_commits_exist(render_range, self.args.render_conformance_tests) + # A regeneration does not reset the code, so only the other renders can remove one. + if not self.is_regenerate: + plain_module.ensure_render_from_keeps_regenerations(render_range[0]) has_any_required_module_changed = False if not self.args.render_machine_graph and plain_module.required_modules and not is_render_choice_module: @@ -121,10 +130,15 @@ def _render_module( self.codeplainAPI, plain_module.module_memory_folder, ) + # Only the module that owns the target functionality is being regenerated. Required modules + # are rendered in full, so they must not take the regenerate path. + is_regenerate = self.is_regenerate and render_range is not None + render_context = self._build_render_context_for_module( plain_module, memory_manager, render_range, + is_regenerate, ) code_renderer = CodeRenderer(render_context) @@ -139,7 +153,11 @@ def _render_module( code_renderer.render_context.event_bus.publish(RenderFailed(error_message=error_message)) return False, True - plain_module.save_module_metadata() + # save_module_metadata records "the whole module matches the current spec". After a + # regenerate only one functionality was rendered, and FinishFunctionalRequirement already + # updated its metadata entry, so writing the whole baseline would claim too much. + if not is_regenerate: + plain_module.save_module_metadata() self.loaded_modules.append(plain_module) diff --git a/partial_rendering.py b/partial_rendering.py index d6eb712a..01803da0 100644 --- a/partial_rendering.py +++ b/partial_rendering.py @@ -234,6 +234,26 @@ def change_is_only_future_work( return frids.index(partial_start.frid) > frids.index(plain_module_render_state.last_render_frid) +def _keep_regenerations(choice: RenderChoice) -> RenderChoice: + """Start the choice at an earlier regenerated functionality when its render would remove that regeneration. + + The render resets the code to the functionality before its start. That reset removes a regeneration + made after it, so the render must start at the regenerated functionality. This renders again the + functionalities after it, so the user must confirm the choice. + """ + assert choice.module is not None and choice.render_range is not None + regenerated_frid = choice.module.get_regenerated_frid_removed_by_render_from(choice.render_range[0]) + if regenerated_frid is None: + return choice + return RenderChoice( + module=choice.module, + render_range=plain_spec.get_render_range_from(regenerated_frid, choice.module.plain_source), + choice_type="render_from_regenerated", + wipe_later_modules=choice.wipe_later_modules, + is_destructive=True, + ) + + def _resume_render_choice( plain_module: PlainModule, plain_module_render_state: PlainModuleRenderState, @@ -258,10 +278,12 @@ def _resume_render_choice( next_frid, next_module = plain_module.get_next_frid(pr.last_render_frid, pr.last_render_module.module_name) render_range = plain_spec.get_render_range_from(next_frid, next_module.plain_source) - return RenderChoice( - module=next_module, - render_range=render_range, - choice_type="continue_from_frid", + return _keep_regenerations( + RenderChoice( + module=next_module, + render_range=render_range, + choice_type="continue_from_frid", + ) ) next_module = plain_module.get_next_module(pr.last_render_module.module_name) or pr.last_render_module @@ -323,12 +345,14 @@ def get_render_choices( # module's earlier (unchanged) functionalities. render_range = plain_spec.get_render_range_from(partial_start.frid, partial_start.module.plain_source) render_choices.append( - RenderChoice( - module=partial_start.module, - render_range=render_range, - choice_type="render_from_change", - wipe_later_modules=len(all_affected_modules) > 1, - is_destructive=True, + _keep_regenerations( + RenderChoice( + module=partial_start.module, + render_range=render_range, + choice_type="render_from_change", + wipe_later_modules=len(all_affected_modules) > 1, + is_destructive=True, + ) ) ) diff --git a/plain2code.py b/plain2code.py index 126b354a..a973d340 100644 --- a/plain2code.py +++ b/plain2code.py @@ -339,10 +339,23 @@ def render( # noqa: C901 event_bus: EventBus, default_log_level: str = "INFO", ): - # Compute render range from either --render-range or --render-from + # Compute render range from either --render-range, --render-from, or --regenerate render_range = None - if args.render_range or args.render_from: - render_range = plain_spec.compute_render_range(args, plain_module.plain_source) + is_regenerate = False + if args.render_range or args.render_from or args.regenerate: + if args.regenerate: + frids = list(plain_spec.get_frids(plain_module.plain_source)) + if args.regenerate not in frids: + raise InvalidFridArgument( + f"Cannot regenerate functionality {args.regenerate} because module {plain_module.module_name} " + f"has no such functionality. Its functionalities are: {', '.join(frids)}." + ) + render_range = plain_spec.get_render_range( + args.regenerate + "," + args.regenerate, plain_module.plain_source + ) + is_regenerate = True + else: + render_range = plain_spec.compute_render_range(args, plain_module.plain_source) run_state.user_email = _check_connection(codeplainAPI) @@ -369,7 +382,7 @@ def render( # noqa: C901 module.reconcile_metadata_with_git() render_choice = None - if render_range is None: + if render_range is None and not is_regenerate: plain_module_render_state = get_plain_module_render_state(plain_module, args.render_conformance_tests) if plain_module_render_state is not None: render_choices = get_render_choices(plain_module, plain_module_render_state, args.force_render) @@ -418,6 +431,7 @@ def render( # noqa: C901 event_bus, stop_event=stop_event, enter_pause_event=enter_pause_event, + is_regenerate=is_regenerate, ) render_error: list[Exception] = [] diff --git a/plain2code_arguments.py b/plain2code_arguments.py index e9e783f2..b1ff7f14 100644 --- a/plain2code_arguments.py +++ b/plain2code_arguments.py @@ -346,6 +346,12 @@ def create_parser(): help="Continue generation starting from this specific functionality (e.g. `add-task`). " "The functionality with this ID will be included in the output. The functionality ID must match one of the functionalities in your plain file.", ) + render_range_group.add_argument( + "--regenerate", + type=frid_string, + help="Regenerate a single already-rendered functionality (e.g. `add-task`). " + "The functionality ID must match one of the functionalities in your plain file.", + ) _add_arg( parser, diff --git a/plain_modules.py b/plain_modules.py index ba07e5d5..93c03a71 100644 --- a/plain_modules.py +++ b/plain_modules.py @@ -360,6 +360,11 @@ def update_frid_in_module_metadata(self, frid: str) -> None: functionalities.append(functionality) metadata[MODULE_FUNCTIONALITIES] = functionalities + # source_hash claims the code implements the whole spec, so only stamp it when that is + # true. Regenerating one functionality leaves the other functionalities as they were. + if self._metadata_matches_spec(metadata): + metadata["source_hash"] = self.get_module_source_hash() + metadata_utils.write_metadata(self.module_metadata_path(), metadata) def _unique_chain(self) -> list[tuple[str, dict]]: @@ -379,6 +384,13 @@ def _unique_chain(self) -> list[tuple[str, dict]]: chain.append((module.module_name, module.plain_source)) return chain + def _metadata_matches_spec(self, metadata: dict) -> bool: + """Whether the given metadata already describes the module's current spec.""" + if metadata.get(MODULE_FUNCTIONALITIES) != self._get_module_functional_requirements(): + return False + + return metadata.get("non_functional_source_hash") == self.get_module_non_functional_source_hash() + def get_module_source_hash(self) -> str: return plain_spec.get_hash_value([self.plain_source] + self.resources_list) @@ -562,6 +574,52 @@ def _ensure_frid_commit_exists( f" codeplain {self.module_name}{plain_file.PLAIN_SOURCE_FILE_EXTENSION} --render-from {frid}" ) + def ensure_frid_can_be_regenerated(self, frid: str) -> None: + """Stop a regeneration of a functionality that was never fully rendered. + + Only a rendered functionality can be generated again. This check runs before the required + modules are rendered, so a regeneration that cannot run uses no credits. + """ + if not os.path.exists(self.module_build_folder) or not git_utils.has_commit_for_frid( + self.module_build_folder, frid, self.module_name + ): + raise MissingPreviousFunctionalitiesError( + f"Cannot regenerate functionality {frid} because it has not been fully rendered yet. " + f"Please render all functionalities first by running:\n" + f" codeplain {self.module_name}{plain_file.PLAIN_SOURCE_FILE_EXTENSION}" + ) + + def get_regenerated_frid_removed_by_render_from(self, start_frid: str) -> str | None: + """The earliest functionality before `start_frid` whose regeneration a render from `start_frid` removes. + + A render from `start_frid` resets the code to the "fully implemented" commit of the functionality + before it. A regeneration made after that commit is removed by the reset. The functionalities from + `start_frid` on render again, so only a regeneration of an earlier functionality is lost. + """ + previous_frid = plain_spec.get_previous_frid(self.plain_source, start_frid) + if previous_frid is None or not os.path.exists(self.module_build_folder): + return None + + regenerated_frids = git_utils.get_frids_regenerated_after_frid( + self.module_build_folder, previous_frid, self.module_name + ) + for frid in plain_spec.get_frids_before(self.plain_source, start_frid): + if frid in regenerated_frids: + return frid + return None + + def ensure_render_from_keeps_regenerations(self, start_frid: str) -> None: + """Stop a render from `start_frid` that would remove the regeneration of an earlier functionality.""" + regenerated_frid = self.get_regenerated_frid_removed_by_render_from(start_frid) + if regenerated_frid is None: + return + raise MissingPreviousFunctionalitiesError( + f"Cannot render from functionality {start_frid} of module {self.module_name}. " + f"Functionality {regenerated_frid} was regenerated, and a render from functionality {start_frid} " + f"would remove that regeneration. Render from functionality {regenerated_frid} instead:\n" + f" codeplain {self.module_name}{plain_file.PLAIN_SOURCE_FILE_EXTENSION} --render-from {regenerated_frid}" + ) + def ensure_previous_frid_commits_exist(self, render_range: list[str], render_conformance_tests: bool) -> None: """ Ensure that all FRID commits before the render_range exist. diff --git a/render_machine/actions/commit_conformance_tests_changes.py b/render_machine/actions/commit_conformance_tests_changes.py index d0296240..21aab3f3 100644 --- a/render_machine/actions/commit_conformance_tests_changes.py +++ b/render_machine/actions/commit_conformance_tests_changes.py @@ -10,9 +10,8 @@ class CommitConformanceTestsChanges(BaseAction): SUCCESSFUL_OUTCOME_IMPLEMENTATION_NOT_UPDATED = "conformance_tests_changes_committed_implementation_not_updated" SUCCESSFUL_OUTCOME_IMPLEMENTATION_UPDATED = "conformance_tests_changes_committed_implementation_updated" - def __init__(self, implementation_code_commit_message: str, conformance_tests_commit_message: str): + def __init__(self, implementation_code_commit_message: str): self.implementation_code_commit_message = implementation_code_commit_message - self.conformance_tests_commit_message = conformance_tests_commit_message def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): implementation_updated = False @@ -26,8 +25,13 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | ) implementation_updated = True + conformance_tests_commit_message = ( + git_utils.FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE + if render_context.is_regenerate + else git_utils.FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE + ) functional_requirement_text = render_context.frid_context.specifications[plain_spec.FUNCTIONAL_REQUIREMENTS][-1] - templated_functional_requirement_finished_commit_msg = self.conformance_tests_commit_message.format( + templated_functional_requirement_finished_commit_msg = conformance_tests_commit_message.format( render_context.frid_context.frid ) formatted_conformance_commit_msg = ( diff --git a/render_machine/actions/exit_with_error.py b/render_machine/actions/exit_with_error.py index 3ecbf92d..645f70ac 100644 --- a/render_machine/actions/exit_with_error.py +++ b/render_machine/actions/exit_with_error.py @@ -18,13 +18,14 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any | render_context.last_error_message = error_message console.error(error_message) - render_context.codeplain_api.fail_functional_requirement( - render_context.frid_context.frid, - module_name=render_context.module_name, - run_state=render_context.run_state, - ) - + # A render can fail before any functionality is started, for example when a regeneration is + # not possible. There is then no functionality to report as failed. if render_context.frid_context is not None: + render_context.codeplain_api.fail_functional_requirement( + render_context.frid_context.frid, + module_name=render_context.module_name, + run_state=render_context.run_state, + ) console.info( f"To continue rendering from the last successfully rendered functionality, " f"provide the --render-from {render_context.frid_context.frid} flag." diff --git a/render_machine/actions/finish_functional_requirement.py b/render_machine/actions/finish_functional_requirement.py index 7439c583..fd287e62 100644 --- a/render_machine/actions/finish_functional_requirement.py +++ b/render_machine/actions/finish_functional_requirement.py @@ -1,16 +1,28 @@ from typing import Any -from render_machine.actions.commit_implementation_code_changes import CommitImplementationCodeChanges +import git_utils +from render_machine.actions.base_action import BaseAction from render_machine.render_context import RenderContext -class FinishFunctionalRequirement(CommitImplementationCodeChanges): +class FinishFunctionalRequirement(BaseAction): SUCCESSFUL_OUTCOME = "functional_requirement_finished" - def execute(self, render_context: RenderContext, previous_action_payload: Any | None): + def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): render_context.plain_module.update_frid_in_module_metadata(render_context.frid_context.frid) - super().execute(render_context, previous_action_payload) + commit_message = ( + git_utils.FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE + if render_context.is_regenerate + else git_utils.FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE + ) + git_utils.add_all_files_and_commit( + render_context.build_folder, + commit_message.format(render_context.frid_context.frid), + render_context.module_name, + render_context.frid_context.frid, + render_context.run_state.render_id, + ) render_context.codeplain_api.finish_functional_requirement( render_context.frid_context.frid, diff --git a/render_machine/actions/fix_unit_tests.py b/render_machine/actions/fix_unit_tests.py index c351a210..39b9d008 100644 --- a/render_machine/actions/fix_unit_tests.py +++ b/render_machine/actions/fix_unit_tests.py @@ -245,8 +245,27 @@ def _functional_requirements_section(render_context: RenderContext, specificatio ) if current: sections.append(f"### Module: {render_context.module_name} (Currently Being Implemented)\n{current[-1]}") + later = FixUnitTests._functionalities_after_regenerated_one(render_context) + if later: + sections.append( + f"### Module: {render_context.module_name} (Already Implemented, for context)\n" + "\n".join(later) + ) return "\n\n".join(sections) + @staticmethod + def _functionalities_after_regenerated_one(render_context: RenderContext) -> list[str]: + """The specs of the functionalities after the regenerated one, as their code implements them. + + A regeneration renders one functionality again, and the functionalities after it keep their + code. Without their specs, the agent can change their code to agree with a wrong unit test. + """ + if not render_context.is_regenerate: + return [] + stored = render_context.plain_module.load_module_metadata()["functionalities"] + stored_frids = [functionality["id"] for functionality in stored] + position = stored_frids.index(render_context.frid_context.frid) + return [functionality["markdown"] for functionality in stored[position + 1 :]] + @staticmethod def _read_script(script: str | None) -> str: if not script: diff --git a/render_machine/actions/prepare_repositories.py b/render_machine/actions/prepare_repositories.py index a4b0b9c0..0cb74293 100644 --- a/render_machine/actions/prepare_repositories.py +++ b/render_machine/actions/prepare_repositories.py @@ -7,12 +7,17 @@ from plain2code_console import console from render_machine.actions.base_action import BaseAction from render_machine.render_context import RenderContext +from render_machine.render_types import RenderError class PrepareRepositories(BaseAction): SUCCESSFUL_OUTCOME = "repositories_prepared" + REGENERATE_NOT_POSSIBLE_OUTCOME = "regenerate_not_possible" def execute(self, render_context: RenderContext, _previous_action_payload: Any | None): + if render_context.is_regenerate: + return self._prepare_regenerate(render_context) + if render_context.render_range is not None and render_context.render_range[0] != plain_spec.get_first_frid( render_context.plain_source_tree ): @@ -85,3 +90,59 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | ) return self.SUCCESSFUL_OUTCOME, None + + def _regenerate_not_possible(self, render_context: RenderContext, error_message: str): + # The error goes back as the outcome, so the state machine runs ExitWithError, which logs it. + # Dispatching the error from inside this action would end the render before that step runs, + # and the user would see no reason. + render_context.last_error_message = error_message + return self.REGENERATE_NOT_POSSIBLE_OUTCOME, RenderError.encode(message=error_message).to_payload() + + def _prepare_regenerate(self, render_context: RenderContext): + frid = render_context.render_range[0] + + if not git_utils.has_commit_for_frid(render_context.build_folder, frid, render_context.module_name): + return self._regenerate_not_possible( + render_context, + f"Cannot regenerate functionality {frid} because it has not been fully rendered yet. " + f"Please render all functionalities first by running: " + f"codeplain {render_context.module_name}.plain", + ) + + render_context.starting_frid = frid + + module_metadata = render_context.plain_module.load_module_metadata() + if not module_metadata or "functionalities" not in module_metadata: + return self._regenerate_not_possible( + render_context, + "module_metadata.json is missing or incomplete. Please re-render the module from the beginning.", + ) + + stored_functionalities = module_metadata["functionalities"] + old_functionality = next((stored for stored in stored_functionalities if stored["id"] == frid), None) + if old_functionality is None: + return self._regenerate_not_possible( + render_context, + f"module_metadata.json holds no functionality {frid}. " + "Please re-render the module from the beginning.", + ) + + render_context.old_frid_spec = old_functionality["markdown"] + # The specs the code on disk currently implements (code-truth). Sent to the API so the + # regeneration prompt shows the old spec for any changed-but-not-yet-regenerated FR, + # keeping specs and code in sync. + render_context.code_functional_requirements = [stored["markdown"] for stored in stored_functionalities] + + if render_context.render_conformance_tests: + conformance_tests_json = render_context.conformance_tests.get_conformance_tests_json( + render_context.module_name + ) + if frid in conformance_tests_json: + old_folder = conformance_tests_json[frid]["folder_name"] + file_utils.delete_folder(old_folder) + del conformance_tests_json[frid] + render_context.conformance_tests.dump_conformance_tests_json( + render_context.module_name, conformance_tests_json + ) + + return self.SUCCESSFUL_OUTCOME, None diff --git a/render_machine/actions/render_conformance_tests.py b/render_machine/actions/render_conformance_tests.py index f3de21c6..92040144 100644 --- a/render_machine/actions/render_conformance_tests.py +++ b/render_machine/actions/render_conformance_tests.py @@ -73,14 +73,18 @@ def _render_conformance_tests(self, render_context: RenderContext): console.debug(f"Storing conformance test files in subfolder {conformance_tests_folder_name}/") - render_context.conformance_tests_running_context.get_conformance_tests_json( + conformance_tests_json = render_context.conformance_tests_running_context.get_conformance_tests_json( render_context.conformance_tests_running_context.current_testing_module_name - )[render_context.conformance_tests_running_context.current_testing_frid] = { + ) + conformance_tests_json[render_context.conformance_tests_running_context.current_testing_frid] = { "folder_name": conformance_tests_folder_name, "functional_requirement": render_context.frid_context.specifications[ plain_spec.FUNCTIONAL_REQUIREMENTS ][-1], } + RenderConformanceTests._keep_specification_order( + conformance_tests_json, list(plain_spec.get_frids(render_context.plain_source_tree)) + ) else: conformance_tests_folder_name = ( render_context.conformance_tests_running_context.get_current_conformance_test_folder_name() @@ -123,6 +127,7 @@ def _render_conformance_tests(self, render_context: RenderContext): render_context.conformance_tests_running_context.current_testing_module_name ), run_state=render_context.run_state, + is_regeneration=render_context.is_regenerate, ) self._warn_about_untraced_scenarios(render_context, traceability) @@ -138,6 +143,19 @@ def _render_conformance_tests(self, render_context: RenderContext): return self.SUCCESSFUL_OUTCOME, None + @staticmethod + def _keep_specification_order(conformance_tests_json: dict, frids: list[str]) -> None: + """Put the entries back in the order of the functionalities in the specification. + + A regeneration, or conformance tests written again, removes the entry and adds it at the end. + A module that requires this one runs these tests in the order of the entries. + """ + ordered = sorted( + conformance_tests_json.items(), key=lambda item: frids.index(item[0]) if item[0] in frids else len(frids) + ) + conformance_tests_json.clear() + conformance_tests_json.update(ordered) + @staticmethod def _warn_about_untraced_scenarios(render_context: RenderContext, traceability: dict) -> None: """Warn about a scenario with no test, a test that implements no scenario, and a scenario diff --git a/render_machine/actions/render_functional_requirement.py b/render_machine/actions/render_functional_requirement.py index 31ab95de..8d33d09a 100644 --- a/render_machine/actions/render_functional_requirement.py +++ b/render_machine/actions/render_functional_requirement.py @@ -79,6 +79,9 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any | render_context.get_required_modules_functionalities(), render_context.should_run_unit_tests(), render_context.run_state, + is_regeneration=render_context.is_regenerate, + old_functional_requirement_text=render_context.old_frid_spec, + code_functional_requirements=render_context.code_functional_requirements, ) except FunctionalRequirementTooComplex as e: error_message = f"The functionality:\n{render_context.frid_context.functional_requirement_text}\n is too complex to be implemented. Please break down the functionality into smaller parts." diff --git a/render_machine/render_context.py b/render_machine/render_context.py index 9bca66ae..ba59dec9 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -50,6 +50,7 @@ def __init__( test_script_timeout: Optional[int] = None, stop_event: Optional[threading.Event] = None, enter_pause_event: Optional[threading.Event] = None, + is_regenerate: bool = False, ): self.codeplain_api: CodeplainAPI = codeplain_api self.memory_manager = memory_manager @@ -73,6 +74,9 @@ def __init__( self.event_bus = event_bus self.stop_event = stop_event self.enter_pause_event = enter_pause_event + self.is_regenerate = is_regenerate + self.old_frid_spec: str | None = None + self.code_functional_requirements: list[str] | None = None self.script_execution_history = ScriptExecutionHistory() self.starting_frid = None self.test_script_timeout = test_script_timeout @@ -87,7 +91,7 @@ def __init__( self.frid_context: Optional[FridContext] = None self.unit_tests_running_context: Optional[UnitTestsRunningContext] = None self.conformance_tests_running_context: Optional[ConformanceTestsRunningContext] = None - # Constants that should remain for a single frid, but possible over multiple rerenderings of the same frid + # Constants that should remain for a single frid, but possible over multiple regenerations of the same frid self.functional_requirements_render_attempts_failed_unit_during_conformance_tests = 0 # Initialize conformance tests utilities. The resolver lets a required module that ships @@ -415,11 +419,13 @@ def _get_next_test_to_run(self): def _has_reached_implementation_frid(self) -> bool: """Check if regression has reached the FRID being implemented.""" ctx = self.conformance_tests_running_context - return ( + if not ( ctx.execution_phase == TestExecutionPhase.RUNNING_REGRESSION and ctx.current_testing_module_name == self.module_name - and (ctx.current_testing_frid is None or ctx.current_testing_frid == ctx.frid_being_implemented) - ) + ): + return False + + return ctx.current_testing_frid is None or ctx.current_testing_frid == ctx.frid_being_implemented def _setup_test_specifications(self): """Load specifications for the current test.""" @@ -544,25 +550,65 @@ def _handle_regression_testing(self): # Get reference to the updated context ctx = self.conformance_tests_running_context + if ( + self.is_regenerate + and ctx.current_testing_module_name == self.module_name + and ctx.current_testing_frid == ctx.frid_being_implemented + ): + # The regenerated functionality was already tested in the initial phase, so move past it. + # The functionalities after it keep their code, so the run continues to the end of the spec + # file. Past the last functionality the frid is None, and the run ends below. + self.conformance_tests_running_context = self._get_next_test_to_run() + ctx = self.conformance_tests_running_context + + if ctx.current_testing_frid is None or not ctx.current_conformance_tests_exist(): + self._end_regression_run() + return + # Set up specs and run test self._setup_test_specifications() - if ctx.current_conformance_tests_exist(): - # Check if this is the implementation FRID (last test to run) - if self._has_reached_implementation_frid(): - # Reached implementation FRID - only re-run it if code changed during regression - if ctx.code_changed_during_regression: - # Code changed - run the implementation FRID again to verify no regression - # After it passes, mark as completed on next iteration - ctx.execution_phase = TestExecutionPhase.COMPLETED - else: - # No code changes - skip re-running implementation FRID, mark as completed immediately - ctx.execution_phase = TestExecutionPhase.COMPLETED - self.machine.dispatch(triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED) - return + # Check if this is the implementation FRID (last test to run) + if not self.is_regenerate and self._has_reached_implementation_frid(): + # Reached implementation FRID - only re-run it if code changed during regression + if ctx.code_changed_during_regression: + # Code changed - run the implementation FRID again to verify no regression + # After it passes, mark as completed on next iteration + ctx.execution_phase = TestExecutionPhase.COMPLETED + else: + # No code changes - skip re-running implementation FRID, mark as completed immediately + ctx.execution_phase = TestExecutionPhase.COMPLETED + self.machine.dispatch(triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED) + return + + self.machine.dispatch(triggers.MARK_CONFORMANCE_TESTS_READY) + + def _end_regression_run(self): + """End the regression run when the next functionality has no conformance tests, or when a + regeneration has walked past the last functionality. + + The functionalities of this module are walked through the spec file, so the walk can reach a + functionality that has none. A project that was rendered only in part has such + functionalities. Nothing is left to run then, and the run must not end without a trigger: + the state machine would stay in the same state and generate conformance tests for that + functionality. + The running context goes back to the functionality being implemented, because the + postprocessing steps look their conformance test folder up by it. + """ + ctx = self.conformance_tests_running_context + ctx.current_testing_module_name = self.module_name + ctx.current_testing_frid = ctx.frid_being_implemented + self._setup_test_specifications() + ctx.execution_phase = TestExecutionPhase.COMPLETED + + if ctx.code_changed_during_regression: + # Code changed during the run, so test the functionality being implemented once more. self.machine.dispatch(triggers.MARK_CONFORMANCE_TESTS_READY) + else: + self.machine.dispatch(triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED) + # TODO why is this in the context and not part of the state machine (in an action) # ========== Main Conformance Test Orchestration ========== def start_conformance_tests_for_frid(self): diff --git a/render_machine/render_types.py b/render_machine/render_types.py index 02ed1da0..446cd800 100644 --- a/render_machine/render_types.py +++ b/render_machine/render_types.py @@ -167,7 +167,10 @@ def get_current_acceptance_scenario(self) -> Optional[dict]: if plain_spec.ACCEPTANCE_TESTS not in self.current_testing_frid_specifications: return None acceptance_tests = self.current_testing_frid_specifications[plain_spec.ACCEPTANCE_TESTS] - if not acceptance_tests or self.acceptance_tests_completed == 0: + # The counter belongs to the functionality being implemented. The specifications can belong + # to another functionality, which can have fewer acceptance tests, so the counter alone is + # not a valid position in this list. + if self.acceptance_tests_completed == 0 or self.acceptance_tests_completed > len(acceptance_tests): return None return acceptance_tests[self.acceptance_tests_completed - 1] diff --git a/render_machine/render_utils.py b/render_machine/render_utils.py index 790ec61e..5d7dac2f 100644 --- a/render_machine/render_utils.py +++ b/render_machine/render_utils.py @@ -26,6 +26,8 @@ def revert_changes_for_frid(render_context): + if render_context.is_regenerate: + return if render_context.frid_context.frid is not None: previous_frid = plain_spec.get_previous_frid(render_context.plain_source_tree, render_context.frid_context.frid) render_context.plain_module.revert_code_to_frid(previous_frid) diff --git a/render_machine/state_machine_config.py b/render_machine/state_machine_config.py index b9679a7f..893b22fe 100644 --- a/render_machine/state_machine_config.py +++ b/render_machine/state_machine_config.py @@ -57,12 +57,9 @@ def get_action_map(self) -> Dict[str, Any]: f"{States.IMPLEMENTING_FRID.value}_{States.PROCESSING_CONFORMANCE_TESTS.value}_{States.POSTPROCESSING_CONFORMANCE_TESTS.value}_{States.CONFORMANCE_TESTS_READY_FOR_SUMMARY.value}": SummarizeConformanceTests(), f"{States.IMPLEMENTING_FRID.value}_{States.PROCESSING_CONFORMANCE_TESTS.value}_{States.POSTPROCESSING_CONFORMANCE_TESTS.value}_{States.CONFORMANCE_TESTS_READY_FOR_COMMIT.value}": CommitConformanceTestsChanges( git_utils.CONFORMANCE_TESTS_PASSED_COMMIT_MESSAGE, - git_utils.FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE, ), f"{States.IMPLEMENTING_FRID.value}_{States.PROCESSING_CONFORMANCE_TESTS.value}_{States.POSTPROCESSING_CONFORMANCE_TESTS.value}_{States.CONFORMANCE_TESTS_READY_FOR_AMBIGUITY_ANALYSIS.value}": AnalyzeSpecificationAmbiguity(), - f"{States.IMPLEMENTING_FRID.value}_{States.FRID_FULLY_IMPLEMENTED.value}": FinishFunctionalRequirement( - git_utils.FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE - ), + f"{States.IMPLEMENTING_FRID.value}_{States.FRID_FULLY_IMPLEMENTED.value}": FinishFunctionalRequirement(), f"{States.IMPLEMENTING_FRID.value}_{States.PROCESSING_CONFORMANCE_TESTS.value}_{States.PROCESSING_UNIT_TESTS.value}_{States.UNIT_TESTS_READY.value}": RunUnitTests(), f"{States.IMPLEMENTING_FRID.value}_{States.PROCESSING_CONFORMANCE_TESTS.value}_{States.PROCESSING_UNIT_TESTS.value}_{States.UNIT_TESTS_FAILED.value}": FixUnitTests(), States.RENDER_COMPLETED.value: CreateDist(), @@ -73,6 +70,7 @@ def get_action_result_triggers_map(self) -> Dict[str, str]: """Get the mapping of action outcomes to state machine triggers.""" return { PrepareRepositories.SUCCESSFUL_OUTCOME: triggers.START_RENDER, + PrepareRepositories.REGENERATE_NOT_POSSIBLE_OUTCOME: triggers.HANDLE_ERROR, RenderFunctionalRequirement.SUCCESSFUL_OUTCOME: triggers.RENDER_FUNCTIONAL_REQUIREMENT, RenderFunctionalRequirement.FUNCTIONAL_REQUIREMENT_TOO_COMPLEX_OUTCOME: triggers.HANDLE_ERROR, RenderFunctionalRequirement.ITERATION_LIMIT_EXCEEDED_OUTCOME: triggers.HANDLE_ERROR, diff --git a/tests/test_conformance_tests_json_order.py b/tests/test_conformance_tests_json_order.py new file mode 100644 index 00000000..f1b933b2 --- /dev/null +++ b/tests/test_conformance_tests_json_order.py @@ -0,0 +1,37 @@ +"""The entries of conformance_tests.json follow the order of the functionalities in the specification. + +A regeneration removes the entry of the functionality and writes it again, so without this the entry +moves to the end. A module that requires this one runs these tests in the order of the entries. +""" + +from render_machine.actions.render_conformance_tests import RenderConformanceTests + +FRIDS = ["run-cli", "add-command", "list-command", "report-command"] + + +def test_a_rewritten_entry_goes_back_to_its_position(): + conformance_tests_json = {"run-cli": 1, "list-command": 3, "report-command": 4, "add-command": 2} + + RenderConformanceTests._keep_specification_order(conformance_tests_json, FRIDS) + + assert list(conformance_tests_json) == FRIDS + + +def test_the_same_object_is_reordered(): + """The running context holds this dict, so it must be reordered in place.""" + conformance_tests_json = {"list-command": 3, "run-cli": 1} + same_object = conformance_tests_json + + RenderConformanceTests._keep_specification_order(conformance_tests_json, FRIDS) + + assert same_object is conformance_tests_json + assert list(same_object) == ["run-cli", "list-command"] + assert same_object["list-command"] == 3 + + +def test_an_entry_the_specification_no_longer_has_goes_last(): + conformance_tests_json = {"removed-command": 9, "add-command": 2, "run-cli": 1} + + RenderConformanceTests._keep_specification_order(conformance_tests_json, FRIDS) + + assert list(conformance_tests_json) == ["run-cli", "add-command", "removed-command"] diff --git a/tests/test_fix_unit_tests_action.py b/tests/test_fix_unit_tests_action.py index d7b18f3c..81a19f4b 100644 --- a/tests/test_fix_unit_tests_action.py +++ b/tests/test_fix_unit_tests_action.py @@ -56,6 +56,7 @@ def render_context(tmp_path, monkeypatch): codeplain_api=None, build_folder=str(build), module_name="m", + is_regenerate=False, run_state=object(), plain_source_tree=plain_source_tree, unittests_script=None, @@ -237,3 +238,36 @@ def test_other_http_errors_on_continue_are_not_swallowed(render_context): render_context.codeplain_api = FakeAPI([requests.exceptions.HTTPError(response=SimpleNamespace(status_code=500))]) with pytest.raises(requests.exceptions.HTTPError): FixUnitTests().execute(render_context, {"previous_unittests_issue": "FAILED"}) + + +def test_a_regeneration_shows_the_functionalities_after_the_regenerated_one(render_context): + """The functionalities after the regenerated one keep their code. The agent must see their specs, as + their code implements them, or it can change that code to agree with a wrong unit test.""" + render_context.is_regenerate = True + render_context.plain_module = SimpleNamespace( + load_module_metadata=lambda: { + "functionalities": [ + {"id": "1", "markdown": "- Old feature."}, + {"id": "2", "markdown": "- New feature, as it was before."}, + {"id": "3", "markdown": "- Later feature."}, + {"id": "4", "markdown": "- Last feature."}, + ] + } + ) + specifications = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, "2")[0] + + section = FixUnitTests._functional_requirements_section(render_context, specifications) + + assert section.endswith( + "### Module: m (Currently Being Implemented)\n- New feature.\n\n" + "### Module: m (Already Implemented, for context)\n- Later feature.\n- Last feature." + ) + assert "as it was before" not in section + + +def test_a_normal_render_shows_no_functionality_after_the_current_one(render_context): + specifications = plain_spec.get_specifications_for_frid(render_context.plain_source_tree, "2")[0] + + section = FixUnitTests._functional_requirements_section(render_context, specifications) + + assert section.endswith("### Module: m (Currently Being Implemented)\n- New feature.") diff --git a/tests/test_fix_unit_tests_conformance_context.py b/tests/test_fix_unit_tests_conformance_context.py index 7d29f07d..8e57c6ab 100644 --- a/tests/test_fix_unit_tests_conformance_context.py +++ b/tests/test_fix_unit_tests_conformance_context.py @@ -108,6 +108,7 @@ def make_render_context(api, build_folder, memory_folder, conformance_tests_runn unit_tests_running_context=UnitTestsRunningContext(fix_attempts=1), plain_source_tree={}, module_name="mod", + is_regenerate=False, required_modules=None, frid_context=SimpleNamespace(frid="1", linked_resources={}, changed_files=set()), get_required_modules_functionalities=lambda: {}, diff --git a/tests/test_git_utils.py b/tests/test_git_utils.py index a234daa9..ed970ed1 100644 --- a/tests/test_git_utils.py +++ b/tests/test_git_utils.py @@ -9,6 +9,7 @@ from git_utils import ( BASE_FOLDER_COMMIT_MESSAGE, FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE, + FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE, REFACTORED_CODE_COMMIT_MESSAGE, add_all_files_and_commit, diff, @@ -495,6 +496,31 @@ def test_get_last_finished_frid_ignores_non_finished_commits(empty_repo): assert get_last_rendered_functionality(empty_repo) == ("module_a", "add-task") +def test_get_last_finished_frid_ignores_regenerated_commits(empty_repo): + """A regenerated commit sits on top of the frontier and must not become the last rendered frid.""" + file_path = Path(empty_repo) / "a.txt" + for frid in ("1", "2", "3"): + file_path.write_text(f"v{frid}") + add_all_files_and_commit( + empty_repo, + FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE.format(frid), + module_name="module_a", + frid=frid, + ) + + # Functionality 1 is rendered again in place, so its commit is the most recent one. + file_path.write_text("v1 again") + add_all_files_and_commit( + empty_repo, + FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE.format("1"), + module_name="module_a", + frid="1", + ) + + # The module is still rendered up to functionality 3. + assert get_last_rendered_functionality(empty_repo) == ("module_a", "3") + + def test_get_last_finished_frid_without_module_name(empty_repo): """Raise InvalidGitRepositoryError when the finished commit omits the module name line.""" file_path = Path(empty_repo) / "a.txt" diff --git a/tests/test_partial_rendering.py b/tests/test_partial_rendering.py index 9b50cbaa..bb8bc8d4 100644 --- a/tests/test_partial_rendering.py +++ b/tests/test_partial_rendering.py @@ -59,6 +59,9 @@ def __init__( self._next_frid = next_frid self.plain_source = _tree(*frids) + def get_regenerated_frid_removed_by_render_from(self, start_frid): + return None + @property def all_required_modules(self): result = [] diff --git a/tests/test_plain_modules.py b/tests/test_plain_modules.py index 2187a289..f5871e34 100644 --- a/tests/test_plain_modules.py +++ b/tests/test_plain_modules.py @@ -60,6 +60,11 @@ def _stored(*frids: str) -> list[dict[str, str]]: return [{"id": frid, "markdown": f"{frid} markdown."} for frid in frids] +def _stale(*frids: str) -> list[dict[str, str]]: + """Metadata entries holding an older markdown than the one the spec file now carries.""" + return [{"id": frid, "markdown": f"old {frid}."} for frid in frids] + + def _write_metadata(module: PlainModule, metadata: dict) -> None: folder = module.get_codeplain_folder() os.makedirs(folder, exist_ok=True) @@ -346,6 +351,93 @@ def test_code_variable_frid_not_flagged_as_change(code_var_module): assert determine_partial_render_start(code_var_module) is None +# -------------------------------------------------------------------------- +# update_frid_in_module_metadata — regenerate of a single functionality +# -------------------------------------------------------------------------- + + +def test_save_module_metadata_restates_every_functionality(solo_module): + """save_module_metadata records the whole module against the current spec. That is why the + regenerate path must not call it: it would restate the functionalities that were not rendered.""" + _write_metadata(solo_module, {"functionalities": ["old one", "old two", "old three"]}) + + solo_module.save_module_metadata() + + metadata = solo_module.load_module_metadata() + assert metadata["functionalities"] == solo_module._get_module_functional_requirements() + assert metadata["source_hash"] == solo_module.get_module_source_hash() + + +def test_update_frid_leaves_other_functionalities_untouched(solo_module): + """Regenerating one functionality must not restate the spec of the others. Their code was not + regenerated, so their metadata entries have to keep the spec the code implements.""" + current = solo_module._get_module_functional_requirements() + stale = _stale("solo-one", "solo-two", "solo-three") + _write_metadata(solo_module, {"functionalities": stale}) + + solo_module.update_frid_in_module_metadata("solo-two") + + metadata = solo_module.load_module_metadata() + assert metadata["functionalities"] == [stale[0], current[1], stale[2]] + + +def test_update_frid_keeps_source_hash_while_another_frid_differs(solo_module): + """source_hash claims the code implements the whole spec, so it must stay stale while the other + functionalities still hold their old spec.""" + _write_metadata( + solo_module, + { + "source_hash": "stale", + "non_functional_source_hash": solo_module.get_module_non_functional_source_hash(), + "functionalities": _stale("solo-one", "solo-two", "solo-three"), + }, + ) + + solo_module.update_frid_in_module_metadata("solo-two") + + assert solo_module.load_module_metadata()["source_hash"] == "stale" + + +def test_update_frid_refreshes_source_hash_when_module_matches_spec(solo_module): + """Once the last outstanding functionality is regenerated, the module matches the spec again and + source_hash is stamped, so no further change is reported.""" + current = solo_module._get_module_functional_requirements() + _write_metadata( + solo_module, + { + "source_hash": "stale", + "non_functional_source_hash": solo_module.get_module_non_functional_source_hash(), + "functionalities": [current[0], _stale("solo-two")[0], current[2]], + }, + ) + + solo_module.update_frid_in_module_metadata("solo-two") + + metadata = solo_module.load_module_metadata() + assert metadata["functionalities"] == current + assert metadata["source_hash"] == solo_module.get_module_source_hash() + + +def test_update_frid_keeps_source_hash_when_non_functional_content_changed(solo_module): + """Every functionality matches, but a definition or requirement changed. The code of the other + functionalities was generated against the old text, so the module is not in sync.""" + current = solo_module._get_module_functional_requirements() + _write_metadata( + solo_module, + { + "source_hash": "stale", + "non_functional_source_hash": "different", + "functionalities": [current[0], _stale("solo-two")[0], current[2]], + }, + ) + + solo_module.update_frid_in_module_metadata("solo-two") + + metadata = solo_module.load_module_metadata() + assert metadata["functionalities"] == current + assert metadata["source_hash"] == "stale" + + # -------------------------------------------------------------------------- # module folder layout # -------------------------------------------------------------------------- diff --git a/tests/test_prepare_repositories_layout.py b/tests/test_prepare_repositories_layout.py index b1d03ac7..ac78e0b8 100644 --- a/tests/test_prepare_repositories_layout.py +++ b/tests/test_prepare_repositories_layout.py @@ -45,6 +45,7 @@ def solo_module(get_test_data_path, tmp_build_folder): def _make_render_context(module: PlainModule, render_conformance_tests: bool) -> SimpleNamespace: return SimpleNamespace( render_range=None, + is_regenerate=False, plain_module=module, required_modules=module.required_modules, build_folder=module.module_build_folder, @@ -244,7 +245,7 @@ def test_archive_with_tests_not_flagged(root_module): def test_render_state_flags_missing_tests_and_offers_rerender(root_module): """A code-only archive (conformance on) surfaces a 'missing_conformance_tests' render state with - a rerender choice and a quit choice.""" + a regenerate choice and a quit choice.""" previous = root_module.required_modules[-1] _init_repo_with_finished_frid(previous.module_build_folder, previous.module_name) _archive_module(previous) # code/ only diff --git a/tests/test_regenerate_checks.py b/tests/test_regenerate_checks.py new file mode 100644 index 00000000..70742e7e --- /dev/null +++ b/tests/test_regenerate_checks.py @@ -0,0 +1,148 @@ +"""The checks that stop a regeneration that cannot run, or a render that would remove a regeneration. + +Both checks run before anything is rendered, and both name the regeneration. The checks for +--render-from would otherwise answer first and talk about starting a render the user did not ask for. +""" + +import os +import tempfile +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import plain2code +from git_utils import ( + FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE, + FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE, + add_all_files_and_commit, + init_git_repo, +) +from partial_rendering import RenderChoice, _keep_regenerations +from plain2code_exceptions import InvalidFridArgument, MissingPreviousFunctionalitiesError +from plain_modules import PlainModule + + +@pytest.fixture +def solo_module(get_test_data_path): + with tempfile.TemporaryDirectory() as build_folder: + yield PlainModule("pr_solo.plain", build_folder, [get_test_data_path("data/partial_rendering")]) + + +def _commit_finished_frid(module: PlainModule, frid: str) -> None: + (Path(module.module_build_folder) / f"frid_{frid}.txt").write_text(f"frid {frid}\n") + add_all_files_and_commit( + module.module_build_folder, + FUNCTIONAL_REQUIREMENT_FINISHED_COMMIT_MESSAGE.format(frid), + module_name=module.module_name, + frid=frid, + ) + + +def _rendered_up_to(module: PlainModule, *frids: str) -> None: + os.makedirs(module.module_build_folder) + init_git_repo(module.module_build_folder, module_name=module.module_name) + for frid in frids: + _commit_finished_frid(module, frid) + + +def test_a_rendered_functionality_can_be_regenerated(solo_module): + _rendered_up_to(solo_module, "solo-one", "solo-two") + + solo_module.ensure_frid_can_be_regenerated("solo-two") + + +def test_a_functionality_after_the_rendered_ones_cannot_be_regenerated(solo_module): + """The previous functionality is not rendered either, but the message is about the regeneration.""" + _rendered_up_to(solo_module, "solo-one") + + with pytest.raises(MissingPreviousFunctionalitiesError) as error: + solo_module.ensure_frid_can_be_regenerated("solo-three") + + assert str(error.value).startswith("Cannot regenerate functionality solo-three because it has not been") + assert "--render-from" not in str(error.value) + + +def test_a_module_that_was_never_rendered_cannot_be_regenerated(solo_module): + with pytest.raises(MissingPreviousFunctionalitiesError) as error: + solo_module.ensure_frid_can_be_regenerated("solo-one") + + assert str(error.value).startswith("Cannot regenerate functionality solo-one because it has not been") + + +def test_an_unknown_func_id_names_the_regeneration(solo_module): + args = SimpleNamespace(render_range=None, render_from=None, regenerate="no-such-id") + + with pytest.raises(InvalidFridArgument) as error: + plain2code.render(solo_module, None, args, None, None) + + assert str(error.value) == ( + "Cannot regenerate functionality no-such-id because module pr_solo has no such functionality. " + "Its functionalities are: solo-one, solo-two, solo-three." + ) + + +def _commit_regenerated_frid(module: PlainModule, frid: str) -> None: + (Path(module.module_build_folder) / f"frid_{frid}.txt").write_text(f"frid {frid} again\n") + add_all_files_and_commit( + module.module_build_folder, + FUNCTIONAL_REQUIREMENT_REGENERATED_COMMIT_MESSAGE.format(frid), + module_name=module.module_name, + frid=frid, + ) + + +def test_a_render_from_that_removes_an_earlier_regeneration_is_stopped(solo_module): + """The render from solo-three resets to the solo-two checkpoint, which is before the regeneration.""" + _rendered_up_to(solo_module, "solo-one", "solo-two", "solo-three") + _commit_regenerated_frid(solo_module, "solo-one") + + with pytest.raises(MissingPreviousFunctionalitiesError) as error: + solo_module.ensure_render_from_keeps_regenerations("solo-three") + + assert str(error.value).startswith( + "Cannot render from functionality solo-three of module pr_solo. Functionality solo-one was regenerated" + ) + assert "--render-from solo-one" in str(error.value) + + +@pytest.mark.parametrize("regenerated, start", [("solo-two", "solo-two"), ("solo-three", "solo-two")]) +def test_a_render_from_that_renders_the_regenerated_functionality_again_can_run(solo_module, regenerated, start): + _rendered_up_to(solo_module, "solo-one", "solo-two", "solo-three") + _commit_regenerated_frid(solo_module, regenerated) + + solo_module.ensure_render_from_keeps_regenerations(start) + + +def test_a_render_from_without_a_regeneration_can_run(solo_module): + _rendered_up_to(solo_module, "solo-one", "solo-two", "solo-three") + + solo_module.ensure_render_from_keeps_regenerations("solo-three") + + +def test_the_earliest_removed_regeneration_is_named(solo_module): + _rendered_up_to(solo_module, "solo-one", "solo-two", "solo-three") + _commit_regenerated_frid(solo_module, "solo-two") + _commit_regenerated_frid(solo_module, "solo-one") + + assert solo_module.get_regenerated_frid_removed_by_render_from("solo-three") == "solo-one" + + +def test_a_continue_choice_that_removes_a_regeneration_starts_at_the_regenerated_functionality(solo_module): + """An interrupted render: solo-one and solo-two are rendered, then solo-one is regenerated.""" + _rendered_up_to(solo_module, "solo-one", "solo-two") + _commit_regenerated_frid(solo_module, "solo-one") + choice = RenderChoice(module=solo_module, render_range=["solo-three"], choice_type="continue_from_frid") + + moved = _keep_regenerations(choice) + + assert moved.choice_type == "render_from_regenerated" + assert moved.render_range == ["solo-one", "solo-two", "solo-three"] + assert moved.is_destructive + + +def test_a_choice_that_removes_no_regeneration_is_not_changed(solo_module): + _rendered_up_to(solo_module, "solo-one", "solo-two") + choice = RenderChoice(module=solo_module, render_range=["solo-three"], choice_type="continue_from_frid") + + assert _keep_regenerations(choice) is choice diff --git a/tests/test_render_context_conformance_regression.py b/tests/test_render_context_conformance_regression.py new file mode 100644 index 00000000..434f3548 --- /dev/null +++ b/tests/test_render_context_conformance_regression.py @@ -0,0 +1,198 @@ +"""Regression testing loop of the conformance tests, for a regenerated functionality. + +The regenerated functionality is tested in the initial phase, so the regression loop moves past it. +Getting the next test moves the frid of the running context in place, and there is no frid after the +last one, so the loop must not advance when the regenerated functionality is the last one. +""" + +import plain_spec +from render_machine import triggers +from render_machine.render_context import RenderContext +from render_machine.render_types import ConformanceTestsRunningContext +from render_machine.render_types import TestExecutionPhase as ExecutionPhase + +FRIDS = ("fr-one", "fr-two", "fr-three", "fr-four") +PLAIN_SOURCE_TREE = {"functional specs": [{"id": frid, "markdown": f"- {frid}"} for frid in FRIDS]} + + +class _RecordingMachine: + def __init__(self): + self.dispatched = [] + + def dispatch(self, trigger): + self.dispatched.append(trigger) + + +def _render_context(current_frid, frid_being_implemented, frids_with_tests, testing_module_name="cli"): + """A RenderContext carrying only what _handle_regression_testing reads. + + _get_next_test_to_run is deliberately NOT stubbed: the real one moves the frid of the running + context in place, which is the behaviour these tests need to cover. + """ + context = ConformanceTestsRunningContext( + current_testing_module_name=testing_module_name, + current_testing_frid=current_frid, + fix_attempts=0, + conformance_tests_json={ + frid: {"folder_name": f"tests_{frid}", "functional_requirement": f"- {frid}"} for frid in frids_with_tests + }, + conformance_tests_render_attempts=0, + current_testing_frid_specifications=None, + should_prepare_testing_environment=False, + frid_being_implemented=frid_being_implemented, + ) + context.execution_phase = ExecutionPhase.RUNNING_REGRESSION + + render_context = RenderContext.__new__(RenderContext) + render_context.is_regenerate = True + render_context.module_name = "cli" + render_context.required_modules = [] + render_context.plain_source_tree = PLAIN_SOURCE_TREE + render_context.machine = _RecordingMachine() + render_context.conformance_tests_running_context = context + return render_context + + +def test_regenerate_of_last_functionality_ends_the_regression_run(): + """The regression loop reaches the regenerated last functionality. It must complete there and keep + the frid, because the postprocessing steps look their folder up by it.""" + render_context = _render_context( + current_frid="fr-three", frid_being_implemented="fr-four", frids_with_tests=["fr-three", "fr-four"] + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED] + assert context.execution_phase is ExecutionPhase.COMPLETED + # The frid must survive. Moving past the last functionality would leave it as None, and the + # conformance test folder lookup would then raise a KeyError of None. + assert context.current_testing_frid == "fr-four" + + +def test_regenerate_of_earlier_functionality_advances_past_it(): + """When the regenerated functionality is not the last one, the loop moves on to the next test.""" + render_context = _render_context( + current_frid="fr-one", frid_being_implemented="fr-two", frids_with_tests=["fr-one", "fr-two", "fr-three"] + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert context.current_testing_frid == "fr-three" + assert context.current_testing_frid_specifications is not None + assert render_context.machine.dispatched == [triggers.MARK_CONFORMANCE_TESTS_READY] + + +def test_regenerate_runs_the_tests_of_the_last_functionality(): + """The functionalities after the regenerated one keep their code, so their tests are run as a + regression check. This includes the last functionality in the spec file.""" + render_context = _render_context( + current_frid="fr-three", frid_being_implemented="fr-two", frids_with_tests=list(FRIDS) + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_CONFORMANCE_TESTS_READY] + assert context.execution_phase is ExecutionPhase.RUNNING_REGRESSION + assert context.current_testing_frid == "fr-four" + + +def test_regenerate_ends_the_regression_run_on_the_regenerated_functionality(): + """After the last functionality, the run ends on the regenerated functionality, because the + postprocessing steps look their conformance test folder up by it.""" + render_context = _render_context( + current_frid="fr-four", frid_being_implemented="fr-two", frids_with_tests=list(FRIDS) + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED] + assert context.execution_phase is ExecutionPhase.COMPLETED + assert context.current_testing_frid == "fr-two" + assert context.current_testing_frid_specifications is not None + + +def test_regenerate_tests_the_regenerated_functionality_again_when_code_changed(): + """When the code changed during the run, the regenerated functionality is tested once more, not the + last functionality in the spec file.""" + render_context = _render_context( + current_frid="fr-four", frid_being_implemented="fr-two", frids_with_tests=list(FRIDS) + ) + render_context.conformance_tests_running_context.code_changed_during_regression = True + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_CONFORMANCE_TESTS_READY] + assert context.execution_phase is ExecutionPhase.COMPLETED + assert context.current_testing_frid == "fr-two" + + +def test_regenerated_functionality_number_of_a_required_module_still_runs(): + """Only the module that owns the regenerated functionality skips it in the regression run. A + required module that happens to have a functionality with the same number must still be tested.""" + render_context = _render_context( + current_frid="fr-one", + frid_being_implemented="fr-two", + frids_with_tests=["fr-one", "fr-two", "fr-three"], + testing_module_name="ledger", + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert context.current_testing_module_name == "ledger" + assert context.current_testing_frid == "fr-two" + assert render_context.machine.dispatched == [triggers.MARK_CONFORMANCE_TESTS_READY] + + +def test_regression_run_ends_when_the_next_functionality_has_no_conformance_tests(): + """A project that was rendered only in part has functionalities without conformance tests. The + regression run must end there, on the functionality being implemented, and report that all + conformance tests passed.""" + # The spec file holds four functionalities, but only 1, 2 and 3 were ever given conformance + # tests. Functionality 2 is the regenerated one, and 3 has just been tested. + render_context = _render_context( + current_frid="fr-three", frid_being_implemented="fr-two", frids_with_tests=["fr-one", "fr-three", "fr-two"] + ) + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_ALL_CONFORMANCE_TESTS_PASSED] + assert context.execution_phase is ExecutionPhase.COMPLETED + # The postprocessing steps look the conformance test folder up by this frid, so it must be the + # functionality being implemented and not the one without conformance tests. + assert context.current_testing_frid == "fr-two" + assert context.current_testing_frid_specifications is not None + + +def test_regression_run_tests_the_implemented_functionality_again_when_code_changed(): + """When the implementation code changed during the run, the functionality being implemented is + tested once more before the run ends.""" + render_context = _render_context( + current_frid="fr-three", frid_being_implemented="fr-two", frids_with_tests=["fr-one", "fr-three", "fr-two"] + ) + render_context.conformance_tests_running_context.code_changed_during_regression = True + + render_context._handle_regression_testing() + + context = render_context.conformance_tests_running_context + assert render_context.machine.dispatched == [triggers.MARK_CONFORMANCE_TESTS_READY] + assert context.execution_phase is ExecutionPhase.COMPLETED + assert context.current_testing_frid == "fr-two" + + +def test_acceptance_test_of_another_functionality_is_not_read_past_its_end(): + """The completed count belongs to the functionality being implemented. Another functionality can + have fewer acceptance tests, so the count is not a valid position in its list.""" + context = _render_context( + current_frid="fr-one", frid_being_implemented="fr-two", frids_with_tests=["fr-one"] + ).conformance_tests_running_context + context.current_testing_frid_specifications = {plain_spec.ACCEPTANCE_TESTS: ["- the only acceptance test"]} + context.acceptance_tests_completed = 2 + + assert context.get_current_acceptance_scenario() is None diff --git a/tests/test_render_stop_reason.py b/tests/test_render_stop_reason.py index 41c69c35..a43873ad 100644 --- a/tests/test_render_stop_reason.py +++ b/tests/test_render_stop_reason.py @@ -14,6 +14,7 @@ import pytest +import git_utils import plain2code_logger from event_bus import EventBus from memory_management import MemoryManager @@ -53,7 +54,7 @@ def workspace(): yield build_folder, unittests_script -def make_render_context(api, build_folder, unittests_script, fixtures_dir): +def make_render_context(api, build_folder, unittests_script, fixtures_dir, render_range=None, is_regenerate=False): plain_module = PlainModule("pr_solo.plain", build_folder, [fixtures_dir]) os.makedirs(plain_module.module_build_folder, exist_ok=True) return RenderContext( @@ -68,12 +69,13 @@ def make_render_context(api, build_folder, unittests_script, fixtures_dir): prepare_environment_script=None, copy_build=False, copy_conformance_tests=False, - render_range=None, + render_range=render_range, render_conformance_tests=False, base_folder=fixtures_dir, run_state=RunState(spec_filename="pr_solo.plain"), event_bus=EventBus(), enter_pause_event=threading.Event(), + is_regenerate=is_regenerate, ) @@ -208,3 +210,26 @@ def test_never_logs_none(self, caplog): assert len(messages) == 1 assert "None" not in messages[0] assert render_context.last_error_message == messages[0] + + +def test_regenerating_a_functionality_that_was_not_rendered_names_the_cause(workspace, fixtures_dir, caplog): + build_folder, unittests_script = workspace + api = FakeCodeplainAPI() + render_context = make_render_context( + api, build_folder, unittests_script, fixtures_dir, render_range=["solo-two"], is_regenerate=True + ) + # A module repository with no functionality rendered yet. + git_utils.init_git_repo(render_context.build_folder, "pr_solo") + code_renderer = CodeRenderer(render_context) + + with caplog.at_level(logging.DEBUG, logger=plain2code_logger.LOGGER_NAME): + code_renderer.run() + + expected_message = ( + "Cannot regenerate functionality solo-two because it has not been fully rendered yet. " + "Please render all functionalities first by running: codeplain pr_solo.plain" + ) + assert render_context.state == States.RENDER_FAILED.value + assert error_log_messages(caplog) == [expected_message] + # The TUI shows the same stop reason as the log. + assert render_context.last_error_message == expected_message diff --git a/tests/test_terminal_attachment.py b/tests/test_terminal_attachment.py index f1bca60a..32f77378 100644 --- a/tests/test_terminal_attachment.py +++ b/tests/test_terminal_attachment.py @@ -82,6 +82,7 @@ def make_args(): build_folder="build", render_range=None, render_from=None, + regenerate=None, force_render=False, headless=False, exit_when_done=False, diff --git a/tui/plain_module_render_choice_tui.py b/tui/plain_module_render_choice_tui.py index f3d206a4..2e4837a6 100644 --- a/tui/plain_module_render_choice_tui.py +++ b/tui/plain_module_render_choice_tui.py @@ -58,6 +58,13 @@ def get_msg_from_choice(self, render_choice: RenderChoice) -> str: f" in module [#5593FF]{render_choice.module.module_name}[/]" f": [#888]{quote}[/]" ) + elif render_choice.choice_type == "render_from_regenerated": + assert render_choice.module is not None + assert render_choice.render_range is not None + return ( + f"Render from regenerated functionality [#5593FF]{render_choice.render_range[0]}[/]" + f" in module [#5593FF]{render_choice.module.module_name}[/]" + ) elif render_choice.choice_type == "rerender_affected" and self.plain_module_render_state.change is not None: all_affected_modules = get_all_affected_modules_from_change( self.plain_module, self.plain_module_render_state