-
-
Notifications
You must be signed in to change notification settings - Fork 14
[#287] Updated FileTrait. #357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes focus on standardizing and clarifying the phrasing of Behat step definitions and related documentation for cookie handling and file management. Step annotations in both code and feature files are updated to use more explicit and consistent English, such as including definite articles ("the", "a") and descriptive wording for managed and unmanaged file operations. Migration and documentation files are updated to reflect these new mappings and step definitions. No changes are made to the underlying logic or exported code entities; all updates are textual and documentation-related. Changes
Suggested labels
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This build fails because of the keyboard.feature failing with the following:
This field is indeed not there. And user.feature with the following:
I haven't touched anything in these tests, so why could they fail? |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
MIGRATION.md (1)
62-63
: Formatting issue in table row.The table formatting is incorrect - missing a trailing pipe and proper column alignment.
-| `NEW` | `Then the downloaded file should be a zip archive containing the files partially named:` | +| `NEW` | `Then the downloaded file should be a zip archive containing the files partially named:` |Add a trailing pipe to the end of line 62 and ensure the table has the correct number of columns.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
63-63: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe(MD055, table-pipe-style)
63-63: Table column count
Expected: 2; Actual: 1; Too few cells, row will be missing data(MD056, table-column-count)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MIGRATION.md
(2 hunks)src/CookieTrait.php
(11 hunks)src/FileTrait.php
(8 hunks)steps.md
(4 hunks)tests/behat/features/cookie.feature
(9 hunks)tests/behat/features/file.feature
(10 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
MIGRATION.md
63-63: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
63-63: Table column count
Expected: 2; Actual: 1; Too few cells, row will be missing data
(MD056, table-column-count)
🔇 Additional comments (13)
src/CookieTrait.php (1)
19-19
: Consistent textual improvements to step definitions.The changes standardize the language in step definitions by explicitly using definite articles like "the" before "name" and "value". These improvements make the step definitions clearer and more consistent throughout the trait.
Also applies to: 28-28, 37-37, 46-46, 55-55, 64-64, 82-82, 91-91, 100-100, 109-109, 118-118
src/FileTrait.php (3)
63-65
: Documentation comment alignment with implementation.The comment has been updated to use plural form to match the step definition annotation below it, ensuring documentation consistency.
192-193
: Updated step definition to use clearer language.Changed from singular to plural form and added "the following" to improve clarity and consistency with other similar steps in the codebase.
235-239
: Improved unmanaged file step definitions.Step definitions for unmanaged files have been updated to use more explicit and descriptive phrasing:
- Added "the" or "an" before "unmanaged file"
- Added "at the URI" for clarity
- Used "should exist/not exist/contain/not contain" consistently
These changes make the steps more readable while maintaining the same functionality.
Also applies to: 255-259, 264-268, 275-279, 286-290, 304-308
MIGRATION.md (2)
16-28
: Added migration mappings for CookieTrait step definitions.The new mapping table provides clear documentation for migrating from v2 to v3 format of CookieTrait step definitions.
64-72
: Added migration mappings for FileTrait step definitions.New mappings clearly document the transition from v2 to v3 format for FileTrait step definitions, providing helpful reference for developers updating their tests.
tests/behat/features/file.feature (4)
4-6
: Updated scenario title and step definitions for managed files.Changed from singular to plural form and added "the following" for consistency with implementation in FileTrait.php.
Also applies to: 11-11
26-26
: Standardized managed file step definitions.All instances of "Given managed file:" have been updated to "Given the following managed files:" for consistency throughout the feature file.
Also applies to: 38-38, 46-46, 58-58, 66-66, 78-78, 86-86, 96-96, 104-104
113-128
: Updated unmanaged file step definitions.The step definitions for unmanaged files have been revised to use more explicit and descriptive phrasing, consistently using "at the URI" and "should exist/contain" patterns.
134-134
: Updated scenario descriptions and steps for error cases.Scenario titles and steps have been updated to match the new step definition format. These changes maintain test functionality while improving readability.
Also applies to: 143-143, 147-149, 157-163, 172-178
tests/behat/features/cookie.feature (1)
6-383
:✅ Verification successful
Consistency: Article usage in cookie steps
All modified lines uniformly insert explicit articles (“the”, “a”) in the Gherkin steps for cookie existence and non‑existence. This is a purely textual update that improves clarity. Please verify that the step annotations insrc/CookieTrait.php
have been updated to exactly match these new phrases; mismatches between feature files and their step definitions will lead toUndefined step
errors.Run the following to confirm alignment between feature steps and their definitions:
🏁 Script executed:
#!/bin/bash # Verify that CookieTrait step annotations match the updated feature phrases rg -n '@Then a cookie with the name' -n src/CookieTrait.php rg -n '@Then a cookie with a name' -n src/CookieTrait.php rg -n '@Then a cookie with the name and the value' -n src/CookieTrait.php rg -n '@Then a cookie with the name and a value containing' -n src/CookieTrait.phpLength of output: 1259
Cookie step definitions verified to match updated feature phrases
All Gherkin steps intests/behat/features/cookie.feature
are covered by matching annotations insrc/CookieTrait.php
. No mismatches detected.Key annotations found in
src/CookieTrait.php
:
- Line 19:
@Then a cookie with the name :name should exist
- Line 28:
@Then a cookie with the name :name and the value :value should exist
- Line 37:
@Then a cookie with the name :name and a value containing :partial_value should exist
- Line 46:
@Then a cookie with a name containing :partial_name should exist
- Line 55:
@Then a cookie with a name containing :partial_name and the value :value should exist
- Line 64:
@Then a cookie with a name containing :partial_name and a value containing :partial_value should exist
- Line 82:
@Then a cookie with the name :name and the value :value should not exist
- Line 91:
@Then a cookie with the name :name and a value containing :partial_value should not exist
- Line 100:
@Then a cookie with a name containing :partial_name should not exist
- Line 109:
@Then a cookie with a name containing :partial_name and the value :value should not exist
- Line 118:
@Then a cookie with a name containing :partial_name and a value containing :partial_value should not exist
All modified feature steps align with these definitions—ready to merge.
steps.md (2)
136-167
: Approve: CookieTrait documentation updated
The documentation for CookieTrait steps now explicitly includes “the”/“a” to mirror the feature files. This enhances consistency across docs and tests. Ensure the examples intests/behat/features/cookie.feature
continue to align.
486-519
: Approve: FileTrait documentation additions
New FileTrait entries (managed/unmanaged file creation and assertions) are clearly described and consistent in phrasing. Confirm thatsrc/FileTrait.php
implements step annotations matching these Gherkin strings (and thattests/behat/features/file.feature
has corresponding scenarios).
@skipper-vp I think it now correctly fails on an old step name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the old step name - see CI output
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.x #357 +/- ##
=======================================
Coverage 79.51% 79.51%
=======================================
Files 30 30
Lines 1484 1484
=======================================
Hits 1180 1180
Misses 304 304 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #287
Summary by CodeRabbit
Documentation
Style
Tests