Skip to content

[Scout] Further enhancements to AI Test Reviews + Scout Best Practices#264097

Merged
csr merged 16 commits intoelastic:mainfrom
csr:enhance-scout-test-review
Apr 20, 2026
Merged

[Scout] Further enhancements to AI Test Reviews + Scout Best Practices#264097
csr merged 16 commits intoelastic:mainfrom
csr:enhance-scout-test-review

Conversation

@csr
Copy link
Copy Markdown
Member

@csr csr commented Apr 17, 2026 •
edited
Loading

Test Reviews (powered by Macroscope + scout-best-practices-reviewer skill):

  • Removed noisy "positive reinforcement" comments
  • Switched to inline comments on specific lines
  • Added severity guidelines to avoid overstating minor issues
  • Scoped reviews to test code only (not framework internals)

Scout Best Practices Docs:

  • Added best practices for auth patterns, test isolation, and loadIfNeeded behavior
  • Clarified when to use global setup hooks vs beforeAll/afterAll
  • Documented selective testing and why tests should live close to the code they cover

@csr csr self-assigned this Apr 17, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026 •
edited
Loading

🔍 Preview links for changed docs

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 17, 2026 •
edited
Loading

Vale Linting Results

Summary: 1 warning, 1 suggestion found

⚠️ Warnings (1)
File Line Rule Message
docs/extend/scout/best-practices.md 757 Elastic.DontUse Don't use 'just'.
💡 Suggestions (1)
File Line Rule Message
docs/extend/scout/best-practices.md 501 Elastic.WordChoice Consider using 'can, might' instead of 'may', unless the term is in the UI.

The Vale linter checks documentation changes against the Elastic Docs style guide.

To use Vale locally or report issues, refer to Elastic style guide for Vale.

@csr csr added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 17, 2026
@csr csr changed the title [Macroscope] Further enhancements to AI-driven Scout Test Reviews [Scout] Further enhancements to AI Test Reviews + Scout Best Practices Apr 17, 2026
csr
csr commented Apr 17, 2026
View reviewed changes
Comment thread docs/extend/scout/best-practices.md
- Within a test, avoid relying on configuration, data, or behavior specific to a single deployment. Test logic should produce the same result locally and on Cloud.
- Run your tests against a real Elastic Cloud project before merging to catch environment-specific surprises early. See [Run tests on Elastic Cloud](./run-tests.md#scout-run-tests-cloud) for setup instructions.

### Keep tests close to the code they test [keep-tests-close-to-source-code]
Copy link
Copy Markdown
Member Author

@csr csr Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New best practice centered around selective testing. We also now mention selective testing in the Scout article (under "benefits" + FAQ).

Copy link
Copy Markdown
Member Author

@csr csr Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmlemeshko - will this selective testing "best practice" be enough? It's a bit generic, let me know if you can come up with something more specific.

@csr csr marked this pull request as ready for review April 17, 2026 15:19
@csr csr requested a review from a team as a code owner April 17, 2026 15:19
@csr csr mentioned this pull request Apr 17, 2026
Merged
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Apr 17, 2026

💚 Build Succeeded

cc @csr

dmlemeshko
dmlemeshko reviewed Apr 17, 2026
View reviewed changes
Comment thread docs/extend/scout/best-practices.md Outdated

### Keep tests close to the code they test [keep-tests-close-to-source-code]

Scout uses selective testing to run only the tests for modules affected by a PR. For this to work, tests must live in the same plugin or package as the code they cover, otherwise changes won't trigger the relevant tests. The full suite still runs post-merge on `kibana-on-merge`.
Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow how this instruction "Scout uses selective testing.." assists to code review and what do we expect it to do. If the focus is on "tests should be in the right place" I would phrase it more like looking into test scenarios and verify they logically belong to the plugin/package they were added it. For API tests you can check routes to be defined in /server directory.

For UI we can actualy mention checking '/public' dir to understand what plugin is doing.

Copy link
Copy Markdown
Member Author

@csr csr Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good point. Updated in 491ac72.

dmlemeshko
dmlemeshko reviewed Apr 17, 2026
View reviewed changes
Comment thread docs/extend/scout/best-practices.md
steliosmavro
steliosmavro approved these changes Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@steliosmavro steliosmavro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left an optional nitpick! 🚀

Comment thread docs/extend/scout/best-practices.md
Comment thread docs/extend/scout.md Outdated

#### Q: What is selective testing? [scout-faq-selective-testing]

In PR builds, Scout automatically detects which modules changed and runs only the relevant tests, reducing CI time.
Copy link
Copy Markdown
Contributor

@steliosmavro steliosmavro Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have anything to link here? Feels like a link for more info is missing , like how to know that my tests are being picked up or something like that 🤔 just a thought.

Copy link
Copy Markdown
Member Author

@csr csr Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more info (and a link to the PR #261510) in d5379be.

@csr csr requested a review from dmlemeshko April 20, 2026 07:54
@csr csr requested a review from steliosmavro April 20, 2026 08:34
@csr csr enabled auto-merge (squash) April 20, 2026 08:43
@csr csr merged commit f9111d3 into elastic:main Apr 20, 2026
16 checks passed
@csr csr deleted the enhance-scout-test-review branch April 20, 2026 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmlemeshko dmlemeshko dmlemeshko approved these changes

@steliosmavro steliosmavro Awaiting requested review from steliosmavro

Assignees

@csr csr

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.5.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@csr @elasticmachine @dmlemeshko @steliosmavro @kibanamachine