Streamline repo scaffolding & re-write & re-enable repo scaffolding tests #639
Conversation
| def _render_autonomy_pyproject_template(project_name: str, authors: str) -> str: | ||
| env = Environment(loader=FileSystemLoader(JINJA_TEMPLATE_FOLDER), autoescape=False) # noqa | ||
| template = env.get_template("repo/autonomy/pyproject.jinja") | ||
|
|
||
| pyproject_path = THIS_REPO_ROOT / "pyproject.toml" | ||
| pyproject = toml.load(pyproject_path) | ||
|
|
||
| build_system = pyproject["build-system"] | ||
| classifiers = pyproject["tool"]["poetry"]["classifiers"] | ||
| python = pyproject["tool"]["poetry"]["dependencies"]["python"] | ||
| current_version = pyproject["tool"]["poetry"]["version"] | ||
| min_minor_version = ".".join(current_version.split(".")[:2]) | ||
| version = f">={min_minor_version}.0,<={current_version}" | ||
| dev_dependencies = pyproject["tool"]["poetry"]["group"]["dev"]["dependencies"] | ||
| black_config = pyproject["tool"]["black"] | ||
|
|
||
| return template.render( | ||
| build_system=toml.dumps(build_system), | ||
| project_name=project_name, | ||
| authors=authors, | ||
| classifiers=" " + ",\n ".join(f'"{c}"' for c in classifiers), | ||
| python=python, | ||
| version=version, | ||
| dev_dependencies=toml.dumps(dev_dependencies, TomlPreserveInlineDictEncoder()), | ||
| black_config=toml.dumps(black_config), | ||
| ) |
There was a problem hiding this comment.
Previously, updating dependencies in adev required a manual two-step process to keep the autonomy scaffold in sync:
- You had to update
pyproject.toml.templateinautonomywhenever a dependency or python version range changed inadev. - You then had to regenerate the
poetry.lockfile after rendering the template, to ensure it matched the updated dependencies. If you didn’t,poetry installwould fail, and the corresponding test would break.
While these failures correctly caught drift between the scaffolded repo and adev, the workflow was tedious. On top of that, the .template still hardcoded things like the adev version and tool configurations, and hence only partial consistency was attained.
This PR removes the need for all that: the autonomy pyproject.toml is now rendered from a jinja template that pulls version ranges, tool settings, and dependencies directly from adev's pyproject.toml. As a result, we no longer need to separately verify that the scaffolded repo matches adev: they're guaranteed to be in sync by construction.
| if self.type_of_repo == "autonomy": | ||
| project_name = self.scaffold_kwargs["project_name"] | ||
| authors = self.scaffold_kwargs["author"] | ||
| pyproject_content = _render_autonomy_pyproject_template(project_name, authors) | ||
| (new_repo_dir / "pyproject.toml").write_text(pyproject_content) | ||
| result = subprocess.run( | ||
| ["poetry", "lock"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| cwd=new_repo_dir, | ||
| ) | ||
| if result.returncode != 0: | ||
| msg = f"Failed to lock packages:\n{result.stderr}" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
-
The
poetry.lockis now generated during the scaffolding process. Since it’s no longer included as a template (I removed it, it was previously copied over from the templates directory but never had a.templatesuffix), this slows down the initial setup of an autonomy repo. However:- It will always produce a lockfile, assuming
adev's own locking is valid (though we can’t simply copyadev'spoetry.lock). - We could reintroduce a
poetry.lockas a template to speed up setup, but thepoetry lockstep can stay. It’s acceptable for the lockfile to be regenerated if missing; this shouldn’t cause test failures or CI breaks, as it's merely a convenience, not a correctness issue.
- It will always produce a lockfile, assuming
-
For the Python repo scaffolding, I haven’t added a
pyproject.jinja(out of scope for this PR). The existing template could use an update, but it’s not urgent for our purposes right now
| from aea.configurations.data_types import PublicId | ||
|
|
||
|
|
||
| THIS_REPO_ROOT = Path(__file__).parent.parent |
There was a problem hiding this comment.
This is useful because when working with isolated filesystems, functions like get_repo_root correctly point to the newly scaffolded repo. But there are cases where we still need access to the adev repo, e.g., to fetch the current pyproject.toml for generating a new repo. This allows us to reach back into adev even when the test context is scoped to the isolated filesystem.
| @pytest.fixture | ||
| def test_clean_filesystem(monkeypatch): | ||
| def test_clean_filesystem(): | ||
| """Fixture for invoking command-line interfaces.""" | ||
| with isolated_filesystem() as directory: | ||
| monkeypatch.setenv("PYTHONPATH", directory) | ||
| yield directory |
There was a problem hiding this comment.
test_clean_filesystem is used for testing setting up new repositories. In such case, we can neither know or set the PYTHONPATH a priori
| import subprocess | ||
| from pathlib import Path | ||
|
|
||
| import toml | ||
| import pytest | ||
| from aea.cli.utils.config import get_default_author_from_cli_config | ||
|
|
||
| from auto_dev.utils import change_dir | ||
|
|
||
|
|
||
| class BaseTestRepo: | ||
| """Test scaffolding new repository.""" | ||
|
|
||
| repo_name = "dummy" | ||
| type_of_repo: str | ||
| make_commands: tuple[str] | ||
|
|
||
| @property | ||
| def cli_args(self): | ||
| """CLI arguments.""" | ||
| return ["adev", "repo", "scaffold", self.repo_name, "-t", self.type_of_repo] | ||
|
|
||
| @property | ||
| def parent_dir(self): | ||
| """Parent directory of newly scaffolded repo.""" | ||
| return Path.cwd() | ||
|
|
||
| @property | ||
| def repo_path(self): | ||
| """Path of newly scaffolded repo.""" | ||
| return self.parent_dir / self.repo_name | ||
|
|
||
| def test_repo_new(self, cli_runner, test_clean_filesystem): | ||
| """Test the format command works with the current package.""" | ||
|
|
||
| assert test_clean_filesystem | ||
| runner = cli_runner(self.cli_args) | ||
| result = runner.execute() | ||
| assert result, runner.output | ||
| assert self.repo_path.exists(), f"Repository directory was not created: {self.repo_path}" | ||
| assert (self.repo_path / ".git").exists() | ||
|
|
||
| def test_repo_new_fail(self, cli_runner, test_filesystem): | ||
| """Test the format command works with the current package.""" | ||
|
|
||
| assert test_filesystem | ||
| self.repo_path.mkdir() | ||
| runner = cli_runner(self.cli_args) | ||
| result = runner.execute() | ||
| assert runner.return_code == 1, result.output | ||
|
|
||
| def test_makefile(self, cli_runner, test_clean_filesystem): | ||
| """Test scaffolding of Makefile.""" | ||
| assert test_clean_filesystem | ||
|
|
||
| runner = cli_runner(self.cli_args) | ||
| result = runner.execute(self.cli_args) | ||
| assert result, (runner.stdout, "\n".join(runner.stderr)) | ||
| makefile = self.repo_path / "Makefile" | ||
| assert makefile.exists(), result.output | ||
| assert makefile.read_text(encoding="utf-8") | ||
| assert self.repo_path.exists() | ||
|
|
||
| def test_make_command_executes(self, cli_runner, test_clean_filesystem): | ||
| """Test that the make command can execute properly.""" | ||
| error_messages = {} | ||
| assert test_clean_filesystem | ||
|
|
||
| # Ensure the repository is created before changing directory | ||
| runner = cli_runner(self.cli_args) | ||
| result = runner.execute() | ||
| assert result, runner.output | ||
| assert self.repo_path.exists(), f"Repository directory was not created: {self.repo_path}" | ||
|
|
||
| with change_dir(self.repo_path): | ||
| for command in self.make_commands: | ||
| result = subprocess.run( | ||
| f"make {command}", | ||
| shell=True, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| if runner.return_code != 0: | ||
| error_messages[command] = runner.stderr | ||
| assert not error_messages | ||
|
|
||
|
|
||
| @pytest.mark.skip | ||
| class TestRepoPython(BaseTestRepo): | ||
| """Test scaffolding new python repository.""" | ||
|
|
||
| type_of_repo = "python" | ||
| make_commands = "fmt", "lint", "test" | ||
|
|
||
|
|
||
| @pytest.mark.skip | ||
| class TestRepoAutonomy(BaseTestRepo): | ||
| """Test scaffolding new autonomy repository.""" | ||
|
|
||
| author = get_default_author_from_cli_config() | ||
| type_of_repo = "autonomy" | ||
| make_commands = "fmt", "test", "lint", "hashes" | ||
|
|
||
| def test_gitignore(self, cli_runner, test_clean_filesystem): | ||
| """Test the .gitignore works as expected.""" | ||
|
|
||
| assert test_clean_filesystem | ||
| runner = cli_runner(self.cli_args) | ||
| result = runner.execute() | ||
| assert runner.return_code == 0, runner.output | ||
|
|
||
| packages_folder = self.repo_path / "packages" | ||
| author_packages = packages_folder / self.author | ||
| author_packages.mkdir(parents=True) | ||
|
|
||
| # create files that should be ignored in the authors folder | ||
| for folder in ("protocols", "connections", "skills", "agents", "services"): | ||
| subfolder = author_packages / folder | ||
| subfolder.mkdir() | ||
| (subfolder / "keys.json").write_text("SECRET") | ||
| (subfolder / "ethereum_private_key.txt").write_text("SECRET") | ||
| (subfolder / "my_private_keys").write_text("SECRET") | ||
| (subfolder / "__pycache__").mkdir() | ||
| (subfolder / "__pycache__" / "cachefile").write_text("cache data") | ||
|
|
||
| with change_dir(self.repo_path): | ||
| result = subprocess.run(["git", "status", "--porcelain"], capture_output=True, text=True, check=False) | ||
| assert result.returncode == 0 | ||
|
|
||
| # any other file created in the author's own package directory should be detected | ||
| (author_packages / "some_other_file").write_text("to_be_committed") | ||
| result = subprocess.run(["git", "status", "--porcelain"], capture_output=True, text=True, check=False) | ||
| assert result.returncode == 0 | ||
| assert "packages" in result.stdout | ||
|
|
||
| def test_pyproject_versions( | ||
| self, | ||
| ): | ||
| """Test the pyproject.toml versions are updated.""" | ||
|
|
||
| # We read in the pyproject.toml file and check the versions | ||
| current_pyproject = self.parent_dir / "pyproject.toml" | ||
| repo_pyproject = ( | ||
| self.parent_dir / "auto_dev" / "data" / "repo" / "templates" / "autonomy" / "pyproject.toml.template" | ||
| ) # pylint: disable=line-too-long | ||
|
|
||
| auto_dev_deps = toml.loads(current_pyproject.read_text())["tool"]["poetry"]["dependencies"] | ||
| repo_deps = toml.loads( | ||
| repo_pyproject.read_text().format( | ||
| project_name=self.repo_name, | ||
| author=self.author, | ||
| ) | ||
| )["tool"]["poetry"]["dependencies"] | ||
|
|
||
| errors = [] | ||
| for key in auto_dev_deps: | ||
| if key not in repo_deps: | ||
| continue | ||
| if auto_dev_deps[key] in repo_deps[key]: | ||
| continue | ||
| val = f"{key} New: {auto_dev_deps[key]} old: {repo_deps[key]}" | ||
| errors.append(val) | ||
| assert not errors, errors | ||
| def _test_repo_scaffold(repo_type, make_commands, cli_runner, test_clean_filesystem): | ||
| repo_root = Path(test_clean_filesystem) / "dummy" | ||
| command = ["adev", "repo", "scaffold", repo_root.name, "-t", repo_type] | ||
| runner = cli_runner(command) | ||
| result = runner.execute() | ||
| makefile = repo_root / "Makefile" | ||
|
|
||
| # Verify the basic repository structure exists | ||
| assert result, runner.output | ||
| assert repo_root.exists(), f"Repository {repo_root} does not exist." | ||
| assert (repo_root / ".git").exists(), f".git directory not found in {repo_root}." | ||
| assert makefile.exists(), f"Makefile not found in {repo_root}." | ||
|
|
||
| # Run each make command and collect any errors. | ||
| error_messages = {} | ||
| for cmd in make_commands: | ||
| proc_result = subprocess.run( | ||
| ["make", cmd], | ||
| shell=False, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| cwd=repo_root, | ||
| ) | ||
| if proc_result.returncode != 0: | ||
| error_messages[cmd] = proc_result.stderr | ||
| assert not error_messages, f"Errors encountered in make commands: {error_messages}" | ||
|
|
||
|
|
||
| def test_python_repo(cli_runner, test_clean_filesystem): | ||
| """Test scaffolding a Python repository.""" | ||
|
|
||
| _test_repo_scaffold( | ||
| repo_type="python", | ||
| make_commands=("fmt", "lint", "test"), | ||
| cli_runner=cli_runner, | ||
| test_clean_filesystem=test_clean_filesystem, | ||
| ) | ||
|
|
||
|
|
||
| def test_autonomy_repo(cli_runner, test_clean_filesystem): | ||
| """Test scaffolding an Autonomy repository.""" | ||
|
|
||
| _test_repo_scaffold( | ||
| repo_type="autonomy", | ||
| make_commands=("fmt", "lint", "test", "hashes"), | ||
| cli_runner=cli_runner, | ||
| test_clean_filesystem=test_clean_filesystem, | ||
| ) |
There was a problem hiding this comment.
Rewrote the repo scaffolding tests to eliminate duplication, flakiness, and pytest-xdist interference. Each repo type now has a single test that covers end-to-end scaffolding and Makefile validation. Dropped the “fail if repo exists” test (not worth the cost of invoking isolated_filesystem(copy=True)) and the pyproject.toml version comparison (no longer needed since templates are generated dynamically)
| env = Environment(loader=FileSystemLoader(JINJA_TEMPLATE_FOLDER), autoescape=False) # noqa | ||
| template = env.get_template("repo/autonomy/pyproject.jinja") | ||
|
|
||
| pyproject_path = THIS_REPO_ROOT / "pyproject.toml" |
There was a problem hiding this comment.
consider a more minimal pyproject than adev's for scaffoled repos. (e.g. dev deps in adev pyproject include mkdocs, and may not be appropriate for scaffolded repos)
| [tool.poetry.group.dev.dependencies] | ||
| {{ dev_dependencies }} | ||
|
|
||
| [tool.black] |
There was a problem hiding this comment.
should ruff formatting be preferred?
|
BTW, i noticed that you no longer write the protocol spec to a file called protocol_spec in the generated protocl. Can you please do do this as otherwise a random helper function is required to read the readme to get the spec which is just additional overhead. I had implemented this but it appears you reverted it. |
test_repo.pypoetry.lockand updated itspyproject.tomlrenderingPYTHONPATHmonkeypatching fromtest_clean_filesystem__init__.pyTHIS_REPO_ROOTtoconstants.py