Skip to content

Commit f5b2c2d

Browse files
authored
Give the CI something real to check, and let it start (#1166)
* Replace the committed API key with a placeholder Both data_folder/secrets.yaml and the example next to it shipped a real-looking OpenAI key. It is dead and answers 401, but it should never have been in the repository, and anyone copying the file inherits a broken default instead of a clear instruction to put their own key there. Not rewriting history over it. This repository has thousands of forks, a force push would break every one of them, and the old blob stays reachable by SHA on GitHub regardless, so a rewrite would buy nothing and cost a lot. * Give the CI something real to check, and let it start The workflow had never run a single job. The repository only allowed actions defined inside itself, so actions/checkout was refused before any job was created, and every run ended as a startup failure. That setting now allows GitHub-owned actions and nothing else. Starting was only half of it. The workflow ran pytest against a repository with no tests, which exits 5, so it would have gone red the moment it worked. So there are tests now, and they check the thing that actually breaks for people: the config files this repository ships have to satisfy the validator this repository ships, since copying them is the first step of the setup. One of them refuses a secrets file that holds anything looking like a real key, which is the regression that was sitting in here until today. Two of the tests feed the validator a deliberately broken config and require it to raise, so the suite cannot pass against a validator that accepts anything. Also pins Python to 3.12 rather than tracking whatever is newest, adds an import of the app as a smoke check, and declares inquirer, which main.py imports and which until now arrived only as somebody else's transitive dependency.
1 parent 3e5150c commit f5b2c2d

4 files changed

Lines changed: 80 additions & 5 deletions

File tree

.github/workflows/ci.yml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,18 @@ jobs:
1111

1212
steps:
1313
- name: Checkout code
14-
uses: actions/checkout@v3
14+
uses: actions/checkout@v4
1515

1616
- name: Set up Python
17-
uses: actions/setup-python@v3
17+
uses: actions/setup-python@v5
1818
with:
19-
python-version: '3.x'
19+
python-version: '3.12'
2020

2121
- name: Install dependencies
2222
run: pip install -r requirements.txt
2323

24+
- name: The app has to import on a clean machine
25+
run: python -c "import main"
26+
2427
- name: Run tests
25-
run: pytest
28+
run: pytest -q

conftest.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import sys
2+
from pathlib import Path
3+
4+
# The tests import main.py, which lives at the repository root. pytest puts the
5+
# test file's own directory on sys.path, not this one, so it goes on explicitly.
6+
sys.path.insert(0, str(Path(__file__).parent))

requirements.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ webdriver-manager==4.0.2
2828
pytest
2929
pytest-mock
3030
pytest-cov
31-
undetected-chromedriver==3.5.5
31+
undetected-chromedriver==3.5.5
32+
inquirer

tests/test_shipped_config.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
"""The config files this repository ships have to satisfy the validator it ships.
2+
3+
A broken example config is invisible to whoever breaks it and fatal to every new
4+
user, because copying it is the first thing the setup instructions ask for. The
5+
same goes for a committed credential: one was sitting in both secrets files for
6+
a long time and nothing in here was looking.
7+
"""
8+
9+
from pathlib import Path
10+
11+
import pytest
12+
import yaml
13+
14+
import main
15+
16+
SHIPPED = ["data_folder", "data_folder_example"]
17+
18+
19+
@pytest.mark.parametrize("folder", SHIPPED)
20+
def test_shipped_work_preferences_pass_the_validator(folder):
21+
config = main.ConfigValidator.validate_config(
22+
Path(folder) / "work_preferences.yaml"
23+
)
24+
assert config["positions"], "positions cannot be empty or the run has nothing to do"
25+
26+
27+
@pytest.mark.parametrize("folder", SHIPPED)
28+
def test_shipped_secrets_provide_a_key(folder):
29+
assert main.ConfigValidator.validate_secrets(Path(folder) / "secrets.yaml")
30+
31+
32+
@pytest.mark.parametrize("folder", SHIPPED)
33+
def test_shipped_secrets_are_a_placeholder_and_not_a_real_key(folder):
34+
key = main.ConfigValidator.validate_secrets(Path(folder) / "secrets.yaml")
35+
assert not key.startswith("sk-"), (
36+
f"{folder}/secrets.yaml looks like a real API key rather than a placeholder. "
37+
"Credentials must never be committed here: put yours in the file locally and "
38+
"keep it out of the diff."
39+
)
40+
41+
42+
def test_the_validator_rejects_a_config_with_a_required_key_missing(tmp_path):
43+
"""Without this, every assertion above would also pass against a validator
44+
that accepts anything at all."""
45+
config = yaml.safe_load(
46+
Path("data_folder_example/work_preferences.yaml").read_text(encoding="utf-8")
47+
)
48+
del config["positions"]
49+
broken = tmp_path / "work_preferences.yaml"
50+
broken.write_text(yaml.safe_dump(config), encoding="utf-8")
51+
52+
with pytest.raises(main.ConfigError):
53+
main.ConfigValidator.validate_config(broken)
54+
55+
56+
def test_the_validator_rejects_a_config_with_a_wrong_type(tmp_path):
57+
config = yaml.safe_load(
58+
Path("data_folder_example/work_preferences.yaml").read_text(encoding="utf-8")
59+
)
60+
config["distance"] = "not a number"
61+
broken = tmp_path / "work_preferences.yaml"
62+
broken.write_text(yaml.safe_dump(config), encoding="utf-8")
63+
64+
with pytest.raises(main.ConfigError):
65+
main.ConfigValidator.validate_config(broken)

0 commit comments

Comments
 (0)