Repository navigation
test: record how TargetProcess answers a child's TeamIteration clear - #21
Merged
Merged
Conversation
A parent UserStory and a child Task in the same team and the same sprint, with the child's explicit null sent through clear_team_iteration. TargetProcess unsets the field: the verifying re-read carries "TeamIteration": null - the key present and null, so the "a field the re-read does not carry counts as cleared" lenience is not what passed it - and the method returns the re-read. Recorded because no cassette covered it and the unit suite drives a mock transport, so it can only assert what the client does with a discarded null, never whether TargetProcess discards one. The cascade the method guards against did not reproduce, in either of the two configurations a live run reached. A child created with no Team came back with no TeamIteration at all, so there was nothing to cascade onto; the child recorded here was given the sprint explicitly, so the value it cleared was one TargetProcess had stored for it. The case left untested is a child holding a team assignment but never given a sprint of its own, where the value shown against it could be the parent's rather than its own. TeamIterationCascadeError and the prose describing it are deliberately untouched. This records an observation; whether it narrows the documented behaviour is not a question a fixture should settle on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: man8/targetprocess-py/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds an integration test and VCR cassette for clearing a child task’s ChangesTeam iteration clearing integration coverage
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other 🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
1 participant
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.
What this records
A live recording of what TargetProcess does with a child's explicit
TeamIterationnull while its parent is scheduled — the interaction no cassettecovered, and one the unit suite cannot answer: it drives a mock transport, so it
asserts what the client does with a discarded null, never whether TargetProcess
discards one.
A parent
UserStoryand a childTask, both in the same team and the samesprint. The clear goes through
clear_team_iteration, and TargetProcessunsets the field:
POST /Task/<id>with body{"TeamIteration":null};{"ResourceType": "Task", "Id": <id>, "TeamIteration": null}— the key present and null, so the library's "afield the re-read does not carry at all counts as cleared" lenience is not
what passed it;
clear_team_iterationreturns the narrowed re-read rather than raising.The test asserts exactly that, and nothing more.
What this does not establish
The cascade
TeamIterationCascadeErrorguards against did not reproduce, ineither configuration the live run reached:
Teamcame back with noTeamIterationat all —TargetProcess holds a card's sprint through a team assignment, so there was
nothing for a parent to cascade onto and nothing to clear;
cleared was one TargetProcess had stored against it.
The configuration left untested is a child that holds a team assignment but was
never given a sprint of its own — where a value shown against it could be the
parent's rather than its own, and a null would have nothing to unset. That is
the shape in which a cascade would be expected to bite, and it is the obvious
next thing to record.
The run also never re-read the parent after the child's clear, so the
cassette shows the parent scheduled before the clear and not after. Nothing in
the run writes to the parent in between.
TeamIterationCascadeError,SPEC.mdanddocs/USAGE.mdare deliberatelyuntouched. This adds an observation; whether that observation narrows the
documented behaviour is a design question, not one a fixture should settle on
its own.
Redaction
The cassette went through the existing record-time hooks in
tests/integration/conftest.pywith no new scheme added, and the conventionsare those of the sibling write-path cassettes — placeholder host
example.tpondemand.com,Sanitised <Field>for every name and identityfield, entity Ids recorded as they came off the wire.
Verified over the committed file:
access_token=value,any
authorizationheader, anycookieheader — zero hits each;example.tpondemand.com;Name/Login/FirstName/LastName/FullNamein a responseis the
Sanitised <Field>placeholder. The two real strings are the entitynames the test itself invented, on the request side, which is the documented
rule for a write-path cassette;
CONTRIBUTING.mdand
.coderabbit.yamlboth state — thirteen cassettes already onmaincarry it;
tests/test_cassette_guard.pyreturns clean on it, and was confirmed toactually read this file by planting a real
Namein an in-memory copy andwatching it flag
Items[0].Team.Name.Checks
pytest— 1506 passed (1505 before, plus this test)ruff check,ruff format --checkscripts/check_internal_refs.py,check_large_files.py,check_todos.pyRecording notes
Re-recording needs a team linked to the sandbox project for the duration of the
run, exactly as
test_team_assignment_create_and_deletedoes — TargetProcesswill not put a card in a team's sprint unless that team is assigned to the
card's project. Every other run replays offline and needs no link. Every entity
the run created was deleted by it; the sandbox was swept afterwards and holds no
Taskand no story of this module's prefix.🤖 Generated with Claude Code
Summary by CodeRabbit