Fix execution order of scheduled tests - #80
Conversation
📝 WalkthroughWalkthroughAdds a JSON-HAS-ID-PROPERTY test directive and demo usage, introduces MyAuthProvider types, changes ExecuteScheduledTestsStep to use ITester, adds a test ensuring scheduled tests run before registered tests, documents a retry directive, and bumps version to 1.5.1. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@demo/Tests/002-Cars/001-Add-Car-test.csx`:
- Around line 28-30: The test is treating the directive value as a float and
passing it as the third argument to Equal (which xUnit treats as precision)
instead of asserting it; change tp.GetVariable<float>("IdFromDirective") to an
integral type that matches your JSON ID (e.g.,
tp.GetVariable<long>("IdFromDirective") or int as appropriate), then perform two
explicit assertions: one that requestJson.Id equals responseJson.Id, and a
separate assertion that the directive value equals the same ID (e.g.,
Equal(requestJson.Id, idFromDirective)). Ensure you reference the variables
requestJson.Id, responseJson.Id, idFromDirective and the tp.GetVariable call
when making the change.
In `@docs/docs/directives.md`:
- Around line 97-105: The Purpose sentence for the RETRY-UNTIL-TEST-PASS
directive has a grammatical error; update the Purpose field text for the `##
RETRY-UNTIL-TEST-PASS` directive so it reads "Retries the request until the
defined test passes." (modify the Purpose line associated with the
`RETRY-UNTIL-TEST-PASS` directive).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
demo/.teapie/init.csxdemo/Tests/002-Cars/001-Add-Car-req.httpdemo/Tests/002-Cars/001-Add-Car-test.csxdocs/docs/directives.mdsrc/Directory.Build.propssrc/TeaPie/Testing/ExecuteScheduledTestsStep.cstests/TeaPie.Tests/ApplicationPipelineShould.cs
🔇 Additional comments (5)
src/Directory.Build.props (1)
20-20: LGTM!Patch version bump is appropriate for this bug fix that corrects the execution order of scheduled tests.
demo/.teapie/init.csx (1)
128-147: LGTM!The new
JSON-HAS-ID-PROPERTYdirective correctly follows the established pattern for custom test directives. The try-catch appropriately handles cases where the response body isn't valid JSON or lacks anIdproperty.demo/Tests/002-Cars/001-Add-Car-req.http (1)
14-14: LGTM!The directive usage correctly demonstrates the new
JSON-HAS-ID-PROPERTYdirective registered ininit.csx, extracting theIdproperty from the response and storing it in theIdFromDirectivevariable.src/TeaPie/Testing/ExecuteScheduledTestsStep.cs (1)
6-20: LGTM! This is the core fix for the execution order issue.The refactor from
IRegistratortoITesterand usingExecuteOrSkipTestensures scheduled tests are executed immediately rather than being registered for later execution. The method signature matches the usage, acceptingTestandTestCase?parameters, and is properly awaited.tests/TeaPie.Tests/ApplicationPipelineShould.cs (1)
105-153: Good coverage for scheduled-vs-registered test ordering.This test clearly captures the execution-order regression and validates the intended pipeline behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/docs/directives.md`:
- Line 104: Update the Parameters line for `test-name` to correct grammar and
avoid nested backticks: change "The name of test defined in post-response .csx
script." to "The name of the test defined in the post-response .csx script." and
rewrite the parenthetical example so it doesn't nest backticks (e.g., show the
example with only one level of code markup or escape the inner backticks for the
`tp.Test` usage such as: (for example: tp.Test("test-name", () => { ... })).
Ensure the parameter label `test-name` and the reference to tp.Test remain
present and clear.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
demo/Tests/002-Cars/001-Add-Car-test.csxdocs/docs/directives.md
🚧 Files skipped from review as they are similar to previous changes (1)
- demo/Tests/002-Cars/001-Add-Car-test.csx
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| | **Syntax** | `## RETRY-UNTIL-TEST-PASS: <test-name>` | | ||
| | **Example Usage** | `## RETRY-UNTIL-TEST-PASS: Identifier should be a positive integer` | | ||
| | **Purpose** | Retries the request until the defined test passes. | | ||
| | **Parameters** | `test-name` – The name of test defined in post-response .csx script. (tp.Test(`test-name`, () => )) | |
There was a problem hiding this comment.
Minor grammatical improvements needed in the Parameters field.
The Parameters description has two issues:
- Missing articles: "The name of test defined in post-response .csx script" should include articles for better readability.
- The nested backticks in the parenthetical example may not render correctly in Markdown.
📝 Proposed fix
-| **Parameters** | `test-name` – The name of test defined in post-response .csx script. (tp.Test(`test-name`, () => )) |
+| **Parameters** | `test-name` – The name of a test defined in a post-response .csx script (e.g., `tp.Test("test-name", () => { })`). |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | **Parameters** | `test-name` – The name of test defined in post-response .csx script. (tp.Test(`test-name`, () => )) | | |
| | **Parameters** | `test-name` – The name of a test defined in a post-response .csx script (e.g., `tp.Test("test-name", () => { })`). | |
🤖 Prompt for AI Agents
In `@docs/docs/directives.md` at line 104, Update the Parameters line for
`test-name` to correct grammar and avoid nested backticks: change "The name of
test defined in post-response .csx script." to "The name of the test defined in
the post-response .csx script." and rewrite the parenthetical example so it
doesn't nest backticks (e.g., show the example with only one level of code
markup or escape the inner backticks for the `tp.Test` usage such as: (for
example: tp.Test("test-name", () => { ... })). Ensure the parameter label
`test-name` and the reference to tp.Test remain present and clear.
3 participants
Issue: The property that was loaded within the directive contains an incorrect value. The value is outdated and comes from the last run of the program, not the current one.
Analyzis: Scheduled tests (from directives in .http files) were executed after registered tests instead of before them.
Solution: Scheduled tests now executed immedietly, before registered tests.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.