Crawl/nowakfli/ex15 resilience failure tests#128
Closed
nowakfli wants to merge 21 commits into
Closed
Conversation
GHCP — Crawl: Ex0 bootstrap
GHCP — Crawl: Ex1 repo-orientation
…eline GHCP — Crawl: Ex2 build-test-baseline
GHCP — Crawl: Ex3 tiny-refactor
…sync GHCP — Crawl: Ex4 documentation-sync
Contributor
There was a problem hiding this comment.
Pull request overview
Despite the title focusing on "ex15 resilience failure tests," this PR bundles many distinct exercises into a single change set: resilience handling for BookCollection.load_books(), input validation on add_book, structured logging and a BOOK_APP_SHOW_YEAR display toggle in book_app.py, a list_books micro-benchmark, a cross-platform run_tests.py, a pytest version pin, expanded .gitignore rules for secrets/cert files, and ~14 new ai-track-docs/* notes plus a .copilot-track/crawl/README.md.
Changes:
- Add resilience, validation, structured logging, and a display toggle to the Python book-app sample, with accompanying tests.
- Add a benchmark script, a local test-runner script, and pin
pytest>=8,<9. - Add a large
ai-track-docs/documentation set and broaden.gitignoreto cover key/cert/secret files.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/book-app-project/books.py | Adds OSError fallback in load_books and input validation in add_book. |
| samples/book-app-project/book_app.py | Adds structured logging, BOOK_APP_SHOW_YEAR toggle, and duplicated year validation. |
| samples/book-app-project/utils.py | Extracts parse_year helper and adds docstrings. |
| samples/book-app-project/tests/test_books.py | New tests for validation, logging, toggle, and OSError fallback. |
| samples/book-app-project/tests/test_utils.py | New tests for parse_year and get_book_details. |
| samples/book-app-project/run_tests.py | New local pytest runner script. |
| samples/book-app-project/benchmark_list_books.py | New micro-benchmark for list_books. |
| samples/book-app-project/pyproject.toml | Pins pytest to >=8,<9. |
| .gitignore | Adds patterns for common secret/certificate files. |
| .copilot-track/crawl/README.md | New track overview/README. |
| ai-track-docs/*.md, architecture.mmd | Adds 14 documentation files covering overview, build/test, deps, logging, toggles, resilience, perf, security, static analysis, PR hygiene, backlog, and utils extension guide. |
Comments suppressed due to low confidence (1)
samples/book-app-project/book_app.py:74
handle_addnow performs its own range check (year < 0 or year > 9999) and raises aValueErrorwith the message "Year must be between 0 and 9999.", and then callscollection.add_bookwhich performs the exact same range check and raises the same message. This duplicates the validation rule in two layers (and the BACKLOG file you added even calls out this issue as item #5). If the CLI is going to validate, you can let the collection-layer exception bubble up and remove the duplicate check here; otherwise let the collection own the rule and keephandle_addthin. Right now the two will silently drift if either side changes its bounds.
try:
year = int(year_str) if year_str else 0
if year < 0 or year > 9999:
raise ValueError("Year must be between 0 and 9999.")
collection.add_book(title, author, year)
log_operation("add_book", "success", started_at, year=year)
print("\nBook added successfully.\n")
except ValueError as e:
log_operation("add_book", "validation_error", started_at, reason="invalid_year")
print(f"\nError: {e}\n")
Comment on lines
+116
to
+117
| - Input functions return validated data to the main application logic</content> | ||
| <parameter name="filePath">c:\crawl-walk-run\copilot-cli-for-beginners-mnf\ai-track-docs\extending-utils.md No newline at end of file |
Comment on lines
+19
to
+31
| ```bash | ||
| cd samples/book-app-project | ||
| $env:BOOK_APP_SHOW_YEAR="1" | ||
| python book_app.py list | ||
| ``` | ||
|
|
||
| Disabled: | ||
|
|
||
| ```bash | ||
| cd samples/book-app-project | ||
| $env:BOOK_APP_SHOW_YEAR="0" | ||
| python book_app.py list | ||
| ``` |
Comment on lines
+31
to
+47
| └── tests/ | ||
| └── test_books.py # Unit tests for BookCollection | ||
| ``` | ||
|
|
||
| **Key Components:** | ||
| - `BookCollection` — in-memory list with JSON persistence | ||
| - `Book` — dataclass representing a single book | ||
| - CLI handlers — user input/output management | ||
|
|
||
| ## Test Approach | ||
|
|
||
| **Framework:** pytest (Python 3.10+) | ||
|
|
||
| **Test Coverage:** | ||
| - `samples/book-app-project/tests/test_books.py` — unit tests for `BookCollection` | ||
| - Uses monkeypatch fixture to isolate tests with temp data files | ||
| - Covers: add, list, find, mark-as-read, invalid operations |
Comment on lines
+32
to
+34
| except OSError as error: | ||
| print(f"Warning: could not read data file ({error}). Starting with empty collection.") | ||
| self.books = [] |
Comment on lines
+8
to
+9
| logging.basicConfig(level=logging.INFO, format="%(message)s") | ||
| logger = logging.getLogger("book_app") |
Comment on lines
+102
to
+115
| def test_handle_add_logs_structured_success(monkeypatch, caplog): | ||
| import book_app | ||
|
|
||
| book_app.collection = BookCollection() | ||
|
|
||
| answers = iter(["Structured Logging", "Logger Author", "2024"]) | ||
| monkeypatch.setattr('builtins.input', lambda _: next(answers)) | ||
|
|
||
| with caplog.at_level(logging.INFO, logger="book_app"): | ||
| book_app.handle_add() | ||
|
|
||
| assert "op=add_book" in caplog.text | ||
| assert "status=success" in caplog.text | ||
| assert "elapsed_ms=" in caplog.text |
Collaborator
|
Thank you, but this isn’t something we’re looking to as to the course right now so going to close this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.