[rust] Add a skills flag that will install a skills.md file#17364
Conversation
The file will be installed into the repo for people to be able to guide the LLM with good practises.
There was a problem hiding this comment.
Pull request overview
Adds a new Selenium Manager CLI flag to generate a skills.md file in the current working directory, intended to provide Selenium language examples and best practices for guiding LLM usage.
Changes:
- Introduces
--init-skillsCLI flag to createskills.mdand exit. - Adds a new
skillsmodule containing the embedded markdown content and file-writing helper. - Adds an integration test to verify
skills.mdcreation and expected contents.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
rust/src/main.rs |
Adds --init-skills flag handling and calls the skills file writer before normal execution flow. |
rust/src/skills.rs |
Implements the embedded SKILLS_CONTENT markdown and write_skills_file helper. |
rust/src/lib.rs |
Exposes the new skills module from the library crate. |
rust/tests/skills_tests.rs |
Adds an integration test validating skills.md creation and content markers. |
- Consolidate Logger initialization in main.rs - Ensure proper flushing and exit code in early-exit paths - Prevent overwriting existing skills.md file - Use temporary directory for integration tests to avoid state leakage - Fix binary path resolution when tests change current directory
- Change --init-skills to take an optional file name - Default path to skills/skills.md - Fallback to selenium.md if default exists - Create parent directories if they don't exist - Update integration tests to cover new logic
There was a problem hiding this comment.
Pull request overview
Adds a new Selenium Manager CLI flag to initialize a repository “skills” guide file (markdown) containing Selenium best practices, along with integration tests to validate default, fallback, and no-overwrite behaviors.
Changes:
- Add
--init-skills [FILE_NAME]flag that writes a skills guide toskills/skills.mdby default (or a custom path). - Implement
skillsmodule with embedded guide content and safe file creation (no clobbering). - Add integration tests for default creation, fallback naming, and overwrite protection; adjust test binary path resolution for
current_dirusage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/src/main.rs | Adds CLI flag handling and early-exit flow to write the skills file. |
| rust/src/skills.rs | Introduces embedded guide content and write_skills_file helper. |
| rust/src/lib.rs | Exposes the new skills module from the library crate. |
| rust/tests/common.rs | Makes the test binary path absolute so cmd.current_dir(...) won’t break execution. |
| rust/tests/skills_tests.rs | Adds integration coverage for default, fallback, custom name, and no-overwrite cases. |
bonigarcia
left a comment
There was a problem hiding this comment.
I agree with Copilot about not hardcoding the skill content within a Rust variable (SKILLS_CONTENT). But instead of a local MD file read by Rust, I propose creating an MD file with that content, committing it to our GitHub repository, and having Selenium Manager read it via a raw.githubusercontent.com URL. For example, something similar is already done in firefox.rs with this URL:
const DRIVER_VERSIONS_URL: &str = "https://raw.githubusercontent.com/SeleniumHQ/selenium/trunk/common/geckodriver/geckodriver-support.json";
The content of that URL can be easily read in SM with one line:
let content = read_content_from_link(http_client, url)?;
This way, we can maintain the skills independently of the SM release.
|
@bonigarcia I pushed without looking at your comment moving things into a resource that we can ship. Part of me prefers that so that we can be faster at writing since we don't need to be changing this as often as the Firefox code. |
There was a problem hiding this comment.
Pull request overview
Adds a new Selenium Manager CLI option to initialize a repository-local “skills” markdown guide (embedded in the binary) and validates the behavior with new Rust integration tests.
Changes:
- Introduces
--init-skills [FILE_NAME]to write an embedded skills/best-practices document into the current working directory (with fallback naming to avoid overwrites). - Adds a new
skillsmodule embeddingsrc/resources/skills.mdand a helper to write it safely (no clobbering). - Adds integration tests for default, fallback, custom filename, and no-overwrite behaviors; improves test binary path resolution.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/src/main.rs | Adds the --init-skills CLI flow and exits after creating the file. |
| rust/src/skills.rs | Implements file creation with create_new(true) and embeds the markdown content. |
| rust/src/resources/skills.md | New end-user guidance content to be written by the flag. |
| rust/src/lib.rs | Exposes the new skills module from the library crate. |
| rust/tests/skills_tests.rs | New integration tests covering default/fallback/custom/no-overwrite scenarios. |
| rust/tests/common.rs | Makes the resolved test binary path absolute when needed. |
| rust/BUILD.bazel | Ensures Bazel compilation can see the embedded markdown via compile_data. |
|
@AutomatedTester |
|
That is my plan. The documents are going to be somewhat living for the next few iterations |
The file will be installed into the repo for people to be able to guide the LLM with good practises.
🔗 Related Issues
This is a feature from #17326
💥 What does this PR do?
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes