Skip to content

Filter for APM property file in APM tests#144350

Merged
mamazzol merged 7 commits intoelastic:mainfrom
mamazzol:ci-144338
Mar 19, 2026
Merged

Filter for APM property file in APM tests#144350
mamazzol merged 7 commits intoelastic:mainfrom
mamazzol:ci-144338

Conversation

@mamazzol
Copy link
Copy Markdown
Contributor

Inside the tempDir created to store APM properties, there are possibly other files/folders.
Tightening the filter in the test to reduce flakiness.
Closes: #144338

@mamazzol mamazzol requested a review from a team as a code owner March 16, 2026 17:59
@mamazzol mamazzol added >test Issues or PRs that are addressing/adding tests Team:Core/Infra Meta label for core/infra team :Core/Infra/Metrics Metrics and metering infrastructure labels Mar 16, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@mamazzol mamazzol changed the title Ci 144338 Filter for property file in test Mar 16, 2026
@mamazzol mamazzol changed the title Filter for property file in test Filter for APM property file in APM tests Mar 16, 2026
Copy link
Copy Markdown
Contributor

@prdoyle prdoyle left a comment

Choose a reason for hiding this comment

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

Optional simplification

Path configPath;
try (var files = Files.list(tempDir)) {
configPath = files.findFirst().orElseThrow(() -> new AssertionError("expected temp APM config file"));
configPath = files.filter(p -> p.getFileName().toString().matches("\\.elstcapm\\..*\\.tmp"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This regex seems a bit obfuscated. If I understand, it corresponds roughly to this glob:

.elstcapm.*.tmp

(Except the regex allows for slashes where a glob * does not.)

Could we use the following? It seems a little more readable than an escaped regex.

files.filter(p -> p.startsWith(".elstcapm") && p.endsWith(".tmp"))

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 242ed8f8-bff7-44f0-899b-8117b62e0b36

📥 Commits

Reviewing files that changed from the base of the PR and between 40dc1ba and 575c9e6.

📒 Files selected for processing (1)
  • distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/APMJvmOptionsTests.java

📝 Walkthrough

Walkthrough

The change modifies the file selection logic in APMJvmOptionsTests to use a more specific filtering criterion. Instead of selecting any arbitrary file from a temporary directory, the code now filters files to match those with names starting with ".elstcapm" and ending with ".tmp". If no matching files are found, it raises the same assertion error as before. This targeted filtering ensures the test selects the correct configuration file instead of potentially picking a directory.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR directly addresses issue #144338 by filtering the temp directory to select only APM property files (names starting with '.elstcapm' and ending with '.tmp'), preventing the test from attempting to read directories as properties files and resolving the IOException.
Out of Scope Changes check ✅ Passed All changes are scoped to APMJvmOptionsTests.java and directly address the identified test failure; no extraneous modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.

Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present.

@mamazzol mamazzol merged commit f66ba98 into elastic:main Mar 19, 2026
36 checks passed
@mamazzol mamazzol deleted the ci-144338 branch March 19, 2026 12:04
ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Mar 20, 2026
michalborek pushed a commit to michalborek/elasticsearch that referenced this pull request Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Metrics Metrics and metering infrastructure Team:Core/Infra Meta label for core/infra team >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

APMJvmOptionsTests.testAPMAgentAlwaysAttached fails

3 participants