Crawl/nowakfli/ex5 validation negative test#125
Closed
nowakfli wants to merge 12 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
This PR adds input validation and negative-path tests to the Python sample book app, and introduces supporting “AI track” documentation (system overview, build/test steps, perf baseline, and track scaffolding) intended for the Crawl exercise series.
Changes:
- Added
parse_year()and expanded docstrings inutils.py, plus new pytest coverage for utils and CLI invalid-input paths. - Introduced validation for
BookCollection.add_book()andbook_app.handle_add(), with corresponding negative tests. - Added new AI-track documentation and a small micro-benchmark + baseline write-up for
list_books().
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
samples/book-app-project/utils.py |
Adds parse_year() and docstrings for UI helper functions. |
samples/book-app-project/tests/test_utils.py |
New unit tests for parse_year() and get_book_details() input handling. |
samples/book-app-project/tests/test_books.py |
Adds negative tests for add_book() validation and handle_add() invalid inputs. |
samples/book-app-project/books.py |
Adds input validation to BookCollection.add_book(). |
samples/book-app-project/book_app.py |
Adds CLI-level validation for title/author/year in handle_add(). |
samples/book-app-project/benchmark_list_books.py |
Adds a micro-benchmark harness for BookCollection.list_books(). |
ai-track-docs/SYSTEM-OVERVIEW.md |
New repository/system overview documentation (currently needs updates for new tests). |
ai-track-docs/perf-baseline.md |
Documents baseline benchmark results and how to run them. |
ai-track-docs/extending-utils.md |
Guidance on extending utils.py (currently contains trailing artifact text). |
ai-track-docs/build-test.md |
Build/test instructions (currently includes commands/claims that don’t match repo deps/packaging). |
ai-track-docs/architecture.mmd |
Mermaid “architecture” diagram (currently placeholder content). |
.copilot-track/crawl/README.md |
Adds Crawl-track process and repo-structure guidance. |
Comments suppressed due to low confidence (1)
ai-track-docs/SYSTEM-OVERVIEW.md:47
- The “Test Coverage” section doesn’t mention the new
tests/test_utils.py, and the coverage bullet list no longer matches the actual test suite. Please update this list to include the new utils tests (and what they cover).
**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
+7
to
+23
| def run_benchmark(iterations: int = 10000, book_count: int = 1000) -> dict[str, float]: | ||
| collection = BookCollection() | ||
| collection.books = [ | ||
| Book(title=f"Book {index}", author="Benchmark Author", year=2000 + (index % 20)) | ||
| for index in range(book_count) | ||
| ] | ||
|
|
||
| samples = [] | ||
| for _ in range(iterations): | ||
| start = time.perf_counter() | ||
| collection.list_books() | ||
| samples.append((time.perf_counter() - start) * 1_000_000) | ||
|
|
||
| return { | ||
| "iterations": iterations, | ||
| "book_count": book_count, | ||
| "mean_us": statistics.mean(samples), |
| raise ValueError("Author cannot be empty.") | ||
| if not (0 <= year <= 9999): | ||
| raise ValueError("Year must be between 0 and 9999.") | ||
|
|
Comment on lines
33
to
+42
| author = input("Author: ").strip() | ||
| year_str = input("Year: ").strip() | ||
|
|
||
| # Input validation | ||
| if not title: | ||
| print("\nError: Title cannot be empty.\n") | ||
| return | ||
| if not author: | ||
| print("\nError: Author cannot be empty.\n") | ||
| return |
Comment on lines
+24
to
+33
| **Python Book App Structure:** | ||
| ``` | ||
| samples/book-app-project/ | ||
| ├── book_app.py # Main CLI handler (menu, user I/O) | ||
| ├── books.py # BookCollection class + persistence (data.json) | ||
| ├── utils.py # UI helpers (print_menu, get_user_choice, format output) | ||
| ├── data.json # Persistent storage (book list) | ||
| └── tests/ | ||
| └── test_books.py # Unit tests for BookCollection | ||
| ``` |
Comment on lines
+1
to
+5
| graph TD | ||
| A["Placeholder: System Component A"] --> B["Placeholder: Component B"] | ||
| B --> C["Placeholder: Component C"] | ||
| C --> D["Database"] | ||
| A --> E["External Service"] |
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
+22
to
+28
| Install the project with dev dependencies: | ||
| ```bash | ||
| pip install -e . | ||
| ``` | ||
|
|
||
| This installs the `book-app` package in editable mode and includes `pytest`. | ||
|
|
|
|
||
| ### Run Tests with Coverage Report | ||
| ```bash | ||
| cd samples/book-app-project |
Comment on lines
+59
to
+60
| git clone https://github.com/<your-username>/copilot-cli-for-beginners-mnf.git | ||
| cd copilot-cli-for-beginners-mnf/samples/book-app-project |
Comment on lines
+46
to
+47
| │ ├── dependencies.md # Dependency policy (added in Ex 7) | ||
| │ └── ... # Additional docs as exercises progress |
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.