Conversation
There was a problem hiding this comment.
Pull request overview
This PR focuses on meeting the “Testing Milestone” by adding/organizing test coverage across backend (pytest) and frontend (Jest/RTL), plus small backend API adjustments to support the tested behaviors and legacy client compatibility.
Changes:
- Added a repo-root test runner script and updated README testing instructions.
- Added new backend unit/behavior tests and a new frontend behavioral test for auth token refresh behavior.
- Updated backend project/branch-related schemas and routes (legacy branch naming support, stricter name validation, and conflict compatibility endpoints).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/run_all_tests.sh |
Adds a single-command runner to execute backend core tests then frontend tests. |
test_results.log |
Adds a captured test run log (currently includes local paths and indicates failures). |
frontend/tests/AuthProvider.behavior.test.tsx |
Adds behavioral coverage for AuthProvider refresh/logout + retry logic. |
backend/tests/test_unit_project_utils.py |
Adds unit tests around raw-SQL project deletion helper execution ordering/params. |
backend/tests/test_behavior_invitation_lifecycle.py |
Adds behavioral tests for invitation state transitions (accept/decline/cancel). |
backend/schemas.py |
Extends BranchCreate to accept legacy name and adds parent_branch_id support. |
backend/routers/projects.py |
Validates/normalizes project & branch names; adjusts commit history behavior for missing branches; adds legacy conflict endpoints. |
README.md |
Rewrites Testing section to document setup and the new single-command test runner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+103
| branch_name: Optional[str] = None | ||
| name: Optional[str] = None | ||
| source_commit_id: Optional[UUID] = None # If None, branches from default branch HEAD | ||
| parent_branch_id: Optional[UUID] = None | ||
|
|
| @model_validator(mode="after") | ||
| def validate_name_present(self): | ||
| if not self.branch_name and not self.name: | ||
| raise ValueError("Either 'branch_name' or legacy 'name' must be provided") |
Comment on lines
+18
to
+84
| """Deletion should run cleanup first, then issue all expected SQL statements.""" | ||
| fake_db = AsyncMock() | ||
| project_id = uuid4() | ||
|
|
||
| cleanup_mock = AsyncMock() | ||
| monkeypatch.setattr("utils.project_utils.cleanup_project_s3", cleanup_mock) | ||
|
|
||
| asyncio.run(delete_project_data(fake_db, project_id)) | ||
|
|
||
| cleanup_mock.assert_awaited_once_with(fake_db, project_id) | ||
|
|
||
| executed_sql = [str(args[0]) for args, _kwargs in fake_db.execute.await_args_list] | ||
|
|
||
| assert len(executed_sql) == 12 | ||
| assert "UPDATE commits SET parent_commit_id = NULL" in executed_sql[0] | ||
| assert "UPDATE branches SET head_commit_id = NULL, parent_branch_id = NULL" in executed_sql[1] | ||
| assert "UPDATE blender_objects SET parent_object_id = NULL" in executed_sql[2] | ||
| assert "DELETE FROM blender_objects" in executed_sql[3] | ||
| assert "DELETE FROM object_locks" in executed_sql[4] | ||
| assert "DELETE FROM merge_conflicts" in executed_sql[5] | ||
| assert "DELETE FROM commits" in executed_sql[6] | ||
| assert "DELETE FROM branches" in executed_sql[7] | ||
| assert "DELETE FROM project_metadata" in executed_sql[8] | ||
| assert "DELETE FROM project_invitations" in executed_sql[9] | ||
| assert "DELETE FROM project_members" in executed_sql[10] | ||
| assert "DELETE FROM projects" in executed_sql[11] | ||
|
|
||
| expected_pid = str(project_id) | ||
| for call_obj in fake_db.execute.await_args_list: | ||
| args = call_obj.args | ||
| kwargs = call_obj.kwargs | ||
|
|
||
| if len(args) >= 2: | ||
| assert args[1] == {"pid": expected_pid} | ||
| elif "params" in kwargs: | ||
| assert kwargs["params"] == {"pid": expected_pid} | ||
| else: | ||
| assert False, f"Missing pid bind params for SQL call: {args[0]}" | ||
|
|
||
|
|
||
| def test_delete_project_data_propagates_cleanup_failure(monkeypatch): | ||
| """If S3 cleanup fails, SQL deletes should not run and the error should bubble up.""" | ||
| fake_db = AsyncMock() | ||
| project_id = uuid4() | ||
|
|
||
| cleanup_mock = AsyncMock(side_effect=RuntimeError("s3 is unavailable")) | ||
| monkeypatch.setattr("utils.project_utils.cleanup_project_s3", cleanup_mock) | ||
|
|
||
| with pytest.raises(RuntimeError, match="s3 is unavailable"): | ||
| asyncio.run(delete_project_data(fake_db, project_id)) | ||
|
|
||
| cleanup_mock.assert_awaited_once_with(fake_db, project_id) | ||
| fake_db.execute.assert_not_called() | ||
|
|
||
|
|
||
| def test_delete_project_data_does_not_commit_or_rollback(monkeypatch): | ||
| """Helper should leave transaction control to callers.""" | ||
| fake_db = AsyncMock() | ||
| project_id = uuid4() | ||
|
|
||
| cleanup_mock = AsyncMock() | ||
| monkeypatch.setattr("utils.project_utils.cleanup_project_s3", cleanup_mock) | ||
|
|
||
| asyncio.run(delete_project_data(fake_db, project_id)) | ||
|
|
||
| fake_db.commit.assert_not_called() | ||
| fake_db.rollback.assert_not_called() |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This was
linked to
issues
Apr 27, 2026
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
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.
Testing Milestone — Unit & Behavioral Tests
Adds comprehensive unit and behavioral test coverage across the backend and frontend to satisfy the Testing Milestone rubric.
Backend (pytest)
Added unit tests for auth utilities, Pydantic schemas, SQLAlchemy models, storage utilities, and project deletion logic
Added behavioral tests for full user workflows: registration → login → project creation → collaboration → invitation lifecycle
Fixed cascading import failures caused by removed MergeConflict model reference in projects.py
Restored branch CRUD routes and added compatibility conflict endpoints for legacy clients
Fixed project deletion FK cycle in project_utils.py
Frontend (Jest + React Testing Library)
Added behavioral tests for AuthProvider token refresh flow
Existing API client and component tests confirmed passing
Infrastructure
Added run_all_tests.sh — single command from repo root to run all core tests
Updated README.md Testing section with commands, file locations, and setup instructions
Result: 321 backend tests pass, 60 frontend tests pass via one command.
closes #101, #102, #104, #105