Added configuration sets to sample tests#273
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR migrates Vividus test configuration from flat properties to a named configuration-set model and updates CI workflows and README examples to use ChangesConfiguration-Set Schema Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 14-16: Remove the stray one-cell table row that contains only "||"
between the REST API tests and Web Application tests rows in the README table;
locate the two table rows labelled "REST API tests" and "Web Application tests"
and delete the extra "||" line so the table remains a valid two-column Markdown
table and resolves markdownlint MD056.
In `@src/main/resources/properties/suite/web_app/suite.properties`:
- Line 5: Remove the global TLS bypass by deleting or commenting out the
property web.driver.chrome.command-line-arguments=--ignore-certificate-errors
from suite.properties; instead make the flag opt-in (e.g., leave the property
empty by default or introduce a separate opt-in property like
web.driver.chrome.allow-insecure=true) and update the Chrome driver setup code
to append --ignore-certificate-errors only when that opt-in property or an
environment/profile override is present so UI tests do not disable certificate
validation globally.
In `@src/main/resources/steps/web_app/ui.steps`:
- Line 4: The step using the locator xpath(//button[contains(.,'Log')]) is too
broad and can match "Logout"; replace it with an exact/normalized text matcher
that targets login variants only (e.g. use normalize-space(text()) equality for
"Log in" and "Login" or an OR between those exact normalized strings). Update
the step that contains the locator string xpath(//button[contains(.,'Log')]) to
use the tightened XPath with normalize-space(text()) comparisons for the login
button.
In `@src/main/resources/story/rest_api/Star` Wars API.story:
- Around line 7-12: The "Verify Luke's homeworld" scenario relies on
`${lukes-homeworld}` defined in another scenario; make it self-contained by
adding the step that sets `lukes-homeworld` inside the same scenario (for
example, perform the HTTP GET for the `people` resource that returns Luke's JSON
and save JSONPath `$.homeworld` to story variable `lukes-homeworld` using the
same "When I save JSON element value from `${response}` by JSON path
`$.homeworld` to story variable `lukes-homeworld`" action) before executing the
HTTP GET for `${lukes-homeworld}`, so the scenario (named "Verify Luke's
homeworld") can run independently.
In `@src/main/resources/story/web_app/Web` app login.story:
- Around line 9-13: Rename the misspelled scenario variable `usename` to
`username` everywhere in this story: update the initialization step that sets
`usename` (the `#{generate(Credentials.username)}` call) to set `username`,
change the login step that uses `${usename}` to `${username}`, and update the
assertion step that expects `Welcome, ${usename}!` to `Welcome, ${username}!` so
all references (initialization, When login to web app, and Then text exists) are
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81dda1f5-44d2-4685-a8d1-25c0d5977e4d
📒 Files selected for processing (12)
.github/workflows/electron-tests.yml.github/workflows/mobile-test-run.yml.github/workflows/test-run.ymlREADME.mdsrc/main/resources/properties/configuration.propertiessrc/main/resources/properties/suite/rest_api/suite.propertiessrc/main/resources/properties/suite/web_app/suite.propertiessrc/main/resources/steps/rest_api/schema.stepssrc/main/resources/steps/web_app/ui.stepssrc/main/resources/story/rest_api/Star Wars API.storysrc/main/resources/story/web_app/Web app login.storysrc/main/resources/story/web_app/Web application.story
💤 Files with no reviewable changes (1)
- src/main/resources/story/web_app/Web application.story
683c282 to
1412a90
Compare
valfirst
left a comment
There was a problem hiding this comment.
There should be at least 3 commits/PRs:
- creation of configuration sets
- new web app tests
- refactoring and new rest api tests
eb36aa0 to
2da9b0f
Compare
| configuration.environments= | ||
| configuration-set.active= | ||
|
|
||
| configuration-set.web-app.profiles=web/headless/chrome |
There was a problem hiding this comment.
CI should use headless Chrome, as headed Chrome is not available by default, BUT the local execution (see README.md) should use headed Chrome to visualise the execution process for users
| configuration-set.electron.profiles=desktop/electron,web/desktop/chrome | ||
| configuration-set.electron.suites=electron | ||
| configuration-set.electron.environments= |
There was a problem hiding this comment.
| configuration-set.electron.profiles=desktop/electron,web/desktop/chrome | |
| configuration-set.electron.suites=electron | |
| configuration-set.electron.environments= | |
| configuration-set.electron-app.profiles=desktop/electron,web/desktop/chrome | |
| configuration-set.electron-app.suites=electron | |
| configuration-set.electron-app.environments= |
859c364 to
4c988ca
Compare
4c988ca to
64ad844
Compare
|
Build is failed |
64ad844 to
a325c25
Compare
Summary by CodeRabbit
Chores
Documentation