-
Notifications
You must be signed in to change notification settings - Fork 4
Enhance testing configuration and coverage measurement #638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…and isolated filesystem paths
| def get_test_cpu_count() -> str: | ||
| """Determine how many CPUs to use for pytest-xdist.""" | ||
| return str(cpu_count()) if os.getenv("CI") else "auto" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on local this ensures not all cores will be used (3/4th I believe), while on the CI we take all that are available
| [tool.coverage.paths] | ||
| source = ["auto_dev", "/tmp/*/auto_dev"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this increased test coverage locally from ~34% to ~53%
We have some issue on CI generating the coverage report tho, it appears (see CI run)
1s
Run MishaKav/pytest-coverage-comment@main
File read successfully "/Users/runner/work/auto_dev/auto_dev/./coverage-report.txt"
Error: Coverage file "/Users/runner/work/auto_dev/auto_dev/./coverage-report.txt" has bad format or wrong data
./coverage-report.txt
Nothing to report
tests/test_protocol.py
Outdated
| HYPOTHESIS_SETTINGS = """ | ||
| from hypothesis import HealthCheck, settings | ||
| settings.register_profile("generated", deadline=1000, suppress_health_check=[HealthCheck.too_slow]) | ||
| settings.load_profile("generated") | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running with pytest-xdist, certain harmless issues incorrectly surface. To avoid modifying the scaffolded codebase in a way that affects production code, we inject a workaround into the generated tests/__init__.py during integration tests to ensure tests pass without impacting the production scaffold
tests/test_protocol.py
Outdated
| result = subprocess.run(command, env=env, check=False, text=True, capture_output=True) | ||
| result = subprocess.run(command, check=False, text=True, capture_output=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed now that monkeypatch.setenv is used in the fixtures
tests/test_repo.py
Outdated
| assert not error_messages | ||
|
|
||
|
|
||
| @pytest.mark.skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the ghosts of a flawed pattern I once endorsed. I’ve since repented; see #639
tests/test_repo.py
Outdated
| make_commands = "fmt", "lint", "test" | ||
|
|
||
|
|
||
| @pytest.mark.skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why skip this entire set of tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was temporary, since these tests were very poorly written and caused interference with other tests. They have been re-enabled in PR #639, which was already merged into this PR, and this is their current state in this PR:
Lines 1 to 55 in 876757d
| """Tests for the click cli.""" | |
| import subprocess | |
| from pathlib import Path | |
| 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, | |
| ) |
Streamline repo scaffolding & re-write & re-enable repo scaffolding tests
PYTHONPATHintest_filesystemfixtures to improve test isolationpytest-xdistsupport inpyproject.tomlfor parallel test executionhypothesis.settingsfor scaffolded protocolspyproject.tomlto include additionaltool.coveragesettings for isolated filesystem pathsget_test_cpu_countinauto_dev/testsbased onos.environ.get('CI')Disabled--> Re-enabled in Streamline repo scaffolding & re-write & re-enable repo scaffolding tests #639test_repo.pytemporarily for stability.Note: PR #639 re-enables
test_repo.py