V2.9.0#931
Open
jmrenouard wants to merge 16 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
documentation/specifications/verbose_execution_timings.md (1)
14-35: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix markdown rendering hiccups before they become “features.”
Line 14 uses an unlabeled fenced block, and Line 35 has a stray
35token. Both will annoy linting and can confuse readers.Suggested patch
- ``` + ```text -------- MyISAM Metrics ---------------------------------------------------------------------------- ... [--] MyISAM Metrics execution time: 0.123s @@ [--] Total Execution Time: 1.789s@@
-35
+5. Timing outputs must be placed before the terminal✔ Terminated successfullymessage.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@documentation/specifications/verbose_execution_timings.mdaround lines 14 -
35, The markdown in the verbose_execution_timings.md file has two rendering
issues that will fail linting. First, the opening code fence on line 14 (before
the MyISAM Metrics example output) is unlabeled and should specify the language
type as text (changetotext). Second, there is a stray35token
appearing near line 35 that has no purpose and should be removed entirely. These
corrections will ensure proper markdown rendering and prevent linting errors.</details> <!-- cr-comment:v1:fd421971b9addd8215c95042 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>mysqltuner.pl (2)</summary><blockquote> `11815-11854`: _🎯 Functional Correctness_ | _🟠 Major_ | _⚡ Quick win_ **Compute health scores before comparing them.** `historical_comparison()` now reads `HealthScore` and `SectionalHealthScore`, but those are only populated later inside `make_recommendations()`. The result is a beautifully formatted trend section full of nothing. Move the comparison after score calculation, or calculate scores before this section runs. <details> <summary>🔁 Proposed sequencing direction</summary> ```diff - \&historical_comparison, \&predictive_capacity_analysis, + \&predictive_capacity_analysis, \&check_replication_advanced, \&check_security_2_0, \&generate_auto_fix_snippets, ); @@ - make_recommendations; # Make recommendations based on stats + make_recommendations; # Calculates health scores and recommendations + historical_comparison; # Now HealthScore/SectionalHealthScore exist dump_result; # Dump result if debug is on ``` </details> Also applies to: 13788-13799 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mysqltuner.pl` around lines 11815 - 11854, The `historical_comparison()` function is attempting to compare `HealthScore` and `SectionalHealthScore` values before they have been calculated and populated. These values are only set later inside the `make_recommendations()` function. Move the comparison blocks for HealthScore (checking defined $result{'HealthScore'}), Total Data Size, and SectionalHealthScore (iterating through 'General', 'Storage', 'Security', 'Replication', 'Modeling') to execute after `make_recommendations()` has been called, ensuring the scores are available for meaningful comparison. ``` </details> <!-- cr-comment:v1:17edc7139a2d87da8e3c1b65 --> --- `13462-13496`: _🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_ **Reject conflicting machine-readable output modes.** If both `--json` and `--yaml` are set, stdout gets JSON followed by YAML while `--outputfile` ends up with YAML only. That’s not an output format; that’s a serialization smoothie. <details> <summary>🚦 Proposed guard</summary> ```diff sub dump_result { + if ( $opt{'json'} && $opt{'yaml'} ) { + print STDERR "ERROR: --json and --yaml are mutually exclusive\n"; + return 1; + } `#debugprint` Dumper( \%result ) if ( $opt{'debug'} ); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mysqltuner.pl` around lines 13462 - 13496, The code allows both json and yaml output modes to be enabled simultaneously, causing conflicting output where both formats get printed to stdout while only YAML is written to the outputfile. Add a guard check before the if ( $opt{'json'} ) block that detects when both $opt{'json'} and $opt{'yaml'} are true, print an error message indicating that these options are mutually exclusive, and return early to prevent conflicting output modes from being processed. ``` </details> <!-- cr-comment:v1:95bc5bbad4fa59707854b475 --> </blockquote></details> </blockquote></details>🧹 Nitpick comments (3)
tests/issue_927.t (1)
1-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the test to match the suite convention.
tests/issue_927.tmisses the repo’stests/**naming pattern. The bug is fine; the filename is the odd one out.As per path instructions,
tests/**files should follow existing naming conventions (test_issue_NNN.t,repro_*.t,unit_*.t).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/issue_927.t` around lines 1 - 240, Rename the test file from `tests/issue_927.t` to follow the repository's naming convention for test files. The current filename `issue_927.t` does not match the established patterns. Rename it to `tests/test_issue_927.t` to conform with the `test_issue_NNN.t` naming pattern used in the test suite.Source: Path instructions
tests/unit_versions.t (2)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate warning suppression detected.
Ah, I see you're really committed to suppressing 'once' warnings. So committed, in fact, that you declared it twice. While I admire the enthusiasm, one
no warnings 'once';is sufficient—Perl got the message the first time.🧹 Proposed fix to remove duplicate
use strict; use warnings; no warnings 'once'; -no warnings 'once'; use Test::More;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit_versions.t` at line 4, The file contains duplicate `no warnings 'once';` statements which is unnecessary and redundant. Search through the tests/unit_versions.t file to locate both occurrences of the `no warnings 'once';` pragma and remove one of them, keeping only a single instance at the top of the file where warning suppression is needed.
103-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using File::Temp for better portability.
Creating temp files directly in
$script_dirassumes write permissions and risks leaving orphaned files if the test crashes before cleanup. Since you're already usingFile::Spec, why not inviteFile::Tempto the party? It's better at this sort of thing.♻️ Proposed refactor using File::Temp
+use File::Temp qw(tempfile); + subtest 'historical_comparison health score' => sub { no warnings 'redefine'; my $captured_trend; @@ -100,8 +101,8 @@ my $compare_json = '{"General":{"Date":"2026-06-15"},"HealthScore":70,"Stats":{"QPS":1.5,"Total Data Size":1000}}'; - my $temp_file = File::Spec->catfile( $script_dir, 'temp_compare.json' ); - open my $tfh, '>', $temp_file or die $!; + my ($tfh, $temp_file) = tempfile( SUFFIX => '.json', UNLINK => 1 ); + # File::Temp already opens the file, no need to open again print $tfh $compare_json; close $tfh; @@ -109,7 +110,6 @@ main::historical_comparison(); - unlink $temp_file; - + # UNLINK => 1 handles cleanup automatically ok( defined $captured_trend,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit_versions.t` around lines 103 - 106, Replace the manual temporary file creation in the test using File::Spec->catfile with File::Temp's tempfile function. Import File::Temp at the top of the file, then replace the block that creates `$temp_file`, opens the file handle `$tfh`, prints `$compare_json`, and closes the handle. Instead use File::Temp's tempfile function to create a temporary file that is automatically managed and cleaned up, which eliminates write permission assumptions and orphaned file risks while improving portability across different systems.🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@mysqltuner.pl`: - Around line 12848-12853: The YAML scalar formatting is not compliant with YAML specifications. The code currently converts undefined values to empty strings instead of using the YAML null representation, and multiline values are wrapped in single quotes without proper indentation for continuation lines. Modify the value handling logic: when a value is undefined (undef), emit it as the YAML null value representation instead of an empty string, and for multiline values that contain newlines, implement proper YAML scalar formatting with indented continuation lines or use YAML literal/folded block scalar syntax to maintain valid YAML structure. Apply these fixes to the scalar wrapping logic in the output generation sections (at lines 12848-12853 and the similar blocks at 12867-12872 and 12877-12882). - Around line 13797-13799: Buffered output is not being flushed before dump_csv_files() is called, causing raw_mysqltuner.txt to miss the analysis output generated before this point when using --dumpdir without --outputfile. Add a flush operation immediately before the dump_csv_files() function call to ensure all buffered output is written to the raw output file, guaranteeing the complete analysis output is captured in raw_mysqltuner.txt. - Around line 4256-4263: The `get_load_average()` function unconditionally reads the local `/proc/loadavg` file without checking whether this is a remote or local audit, which causes it to report the client machine's load average instead of the database server's load average for remote audits using `--host` without SSH or container transport. Add a guard condition at the beginning of the function to check if the audit is remote (e.g., by verifying if `--host` is set without proper SSH/container access), and if so, skip the local `/proc/loadavg` reading to ensure the function only uses transport-appropriate methods for obtaining the server's actual load average. - Around line 13484-13496: The YAML output feature exposes sensitive credentials because _to_yaml is serializing the entire %result hash which contains unfiltered MySQLTuner.options and MySQL Client.Authentication Info containing passwords and SSH credentials. Before calling _to_yaml, create a deep copy of the %result hash and remove or redact all sensitive fields that contain credentials (such as keys matching pass, password, ssh-password, and -p patterns), then pass this sanitized copy to _to_yaml instead of the original %result hash. Apply this same sanitized copy to both the console output via print and the file output via the filehandle to ensure credentials are not leaked in either location. In `@tests/unit_phase13_kpis.t`: - Around line 22-46: The hardcoded filename 'mock_old.json' in the variable $mock_old_file can cause race conditions when tests run in parallel, as multiple test instances may read/write to the same file simultaneously. Replace the hardcoded string with a unique temporary filename using Perl's File::Temp module (such as tempfile() or similar) to ensure each test gets an isolated fixture file. Update both the file creation assignment to $mock_old_file and the cleanup check in the END block to use the dynamically generated temporary filename instead of the string literal 'mock_old.json'. --- Outside diff comments: In `@documentation/specifications/verbose_execution_timings.md`: - Around line 14-35: The markdown in the verbose_execution_timings.md file has two rendering issues that will fail linting. First, the opening code fence on line 14 (before the MyISAM Metrics example output) is unlabeled and should specify the language type as text (change ``` to ```text). Second, there is a stray `35` token appearing near line 35 that has no purpose and should be removed entirely. These corrections will ensure proper markdown rendering and prevent linting errors. In `@mysqltuner.pl`: - Around line 11815-11854: The `historical_comparison()` function is attempting to compare `HealthScore` and `SectionalHealthScore` values before they have been calculated and populated. These values are only set later inside the `make_recommendations()` function. Move the comparison blocks for HealthScore (checking defined $result{'HealthScore'}), Total Data Size, and SectionalHealthScore (iterating through 'General', 'Storage', 'Security', 'Replication', 'Modeling') to execute after `make_recommendations()` has been called, ensuring the scores are available for meaningful comparison. - Around line 13462-13496: The code allows both json and yaml output modes to be enabled simultaneously, causing conflicting output where both formats get printed to stdout while only YAML is written to the outputfile. Add a guard check before the if ( $opt{'json'} ) block that detects when both $opt{'json'} and $opt{'yaml'} are true, print an error message indicating that these options are mutually exclusive, and return early to prevent conflicting output modes from being processed. --- Nitpick comments: In `@tests/issue_927.t`: - Around line 1-240: Rename the test file from `tests/issue_927.t` to follow the repository's naming convention for test files. The current filename `issue_927.t` does not match the established patterns. Rename it to `tests/test_issue_927.t` to conform with the `test_issue_NNN.t` naming pattern used in the test suite. In `@tests/unit_versions.t`: - Line 4: The file contains duplicate `no warnings 'once';` statements which is unnecessary and redundant. Search through the tests/unit_versions.t file to locate both occurrences of the `no warnings 'once';` pragma and remove one of them, keeping only a single instance at the top of the file where warning suppression is needed. - Around line 103-106: Replace the manual temporary file creation in the test using File::Spec->catfile with File::Temp's tempfile function. Import File::Temp at the top of the file, then replace the block that creates `$temp_file`, opens the file handle `$tfh`, prints `$compare_json`, and closes the handle. Instead use File::Temp's tempfile function to create a temporary file that is automatically managed and cleaned up, which eliminates write permission assumptions and orphaned file risks while improving portability across different systems.🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID:
0e5fba80-9343-425a-bf7e-7a9e0ae89621⛔ Files ignored due to path filters (2)
.agent/README.mdis excluded by!.agent/**execution.logis excluded by!**/*.log📒 Files selected for processing (19)
CURRENT_VERSION.txtChangelogPOTENTIAL_ISSUES.mdREADME.fr.mdREADME.it.mdREADME.mdREADME.ru.mdROADMAP.mdUSAGE.mdbuild/check_compliance.plbuild/doc_sync.pydocumentation/specifications/verbose_execution_timings.mddocumentation/specifications/warning_elimination_version_cache.mdmysqltuner.plreleases/v2.9.0.mdtests/issue_927.ttests/unit_phase13_kpis.ttests/unit_versions.ttests/verbose_timing.t
Comment on lines
+4256
to
+4263
| sub get_load_average { | ||
| if ( -f "/proc/loadavg" ) { | ||
| my $content = file2string("/proc/loadavg") // ''; | ||
| if ( $content =~ /^([\d.]+)\s+([\d.]+)\s+([\d.]+)/ ) { | ||
| return ( $1, $2, $3 ); | ||
| } | ||
| } | ||
| my $uptime_output = execute_system_command("uptime") // ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t use the client’s load average for remote audits.
get_load_average() reads local /proc/loadavg before considering transport. For --host without SSH/container, the new KPI can score the DBA’s laptop instead of the database server — a classic case of tuning the wrong machine with confidence.
🧭 Proposed guard
sub get_load_average {
- if ( -f "/proc/loadavg" ) {
+ my $prefix = get_transport_prefix();
+ return () if $prefix eq '' && is_remote();
+
+ if ( $prefix eq '' && -f "/proc/loadavg" ) {
my $content = file2string("/proc/loadavg") // '';
if ( $content =~ /^([\d.]+)\s+([\d.]+)\s+([\d.]+)/ ) {
return ( $1, $2, $3 );
}
}
- my $uptime_output = execute_system_command("uptime") // '';
+ my $uptime_output = execute_system_command("uptime") // '';As per path instructions, recommendations output should remain human-readable and actionable for DBAs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysqltuner.pl` around lines 4256 - 4263, The `get_load_average()` function
unconditionally reads the local `/proc/loadavg` file without checking whether
this is a remote or local audit, which causes it to report the client machine's
load average instead of the database server's load average for remote audits
using `--host` without SSH or container transport. Add a guard condition at the
beginning of the function to check if the audit is remote (e.g., by verifying if
`--host` is set without proper SSH/container access), and if so, skip the local
`/proc/loadavg` reading to ensure the function only uses transport-appropriate
methods for obtaining the server's actual load average.
Source: Path instructions
Comment on lines
+12848
to
+12853
| my $v = $val // ''; | ||
| if ( $v =~ /[:\#\n\'\"]/ or $v eq '' ) { | ||
| $v =~ s/'/''/g; | ||
| $v = "'$v'"; | ||
| } | ||
| $output .= " $v\n"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make YAML scalars actually YAML-shaped.
Multiline values are wrapped in single quotes without indenting continuation lines, so fields populated from command output can produce invalid YAML. Nested undef values are also emitted as '' instead of ~, which changes the JSON/YAML contract for unset options. YAML: because apparently JSON wasn’t enough paperwork.
🧾 Proposed scalar helper
+sub _yaml_scalar {
+ my ( $v, $indent ) = `@_`;
+ return "~" unless defined $v;
+
+ if ( $v =~ /\n/ ) {
+ my $pad = ' ' x ( $indent + 1 );
+ $v =~ s/\r\n?/\n/g;
+ $v =~ s/\n/\n$pad/g;
+ return "|\n$pad$v";
+ }
+
+ if ( $v eq '' || $v =~ /[:\#\'\"\[\]\{\},&*?!|>%-]/ || $v =~ /^(?:yes|no|true|false|null|on|off|~)$/i ) {
+ $v =~ s/'/''/g;
+ return "'$v'";
+ }
+ return $v;
+}
+
- my $v = $val // '';
- if ( $v =~ /[:\#\n\'\"]/ or $v eq '' ) {
- $v =~ s/'/''/g;
- $v = "'$v'";
- }
- $output .= " $v\n";
+ $output .= " " . _yaml_scalar( $val, $indent ) . "\n";Also applies to: 12867-12872, 12877-12882
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysqltuner.pl` around lines 12848 - 12853, The YAML scalar formatting is not
compliant with YAML specifications. The code currently converts undefined values
to empty strings instead of using the YAML null representation, and multiline
values are wrapped in single quotes without proper indentation for continuation
lines. Modify the value handling logic: when a value is undefined (undef), emit
it as the YAML null value representation instead of an empty string, and for
multiline values that contain newlines, implement proper YAML scalar formatting
with indented continuation lines or use YAML literal/folded block scalar syntax
to maintain valid YAML structure. Apply these fixes to the scalar wrapping logic
in the output generation sections (at lines 12848-12853 and the similar blocks
at 12867-12872 and 12877-12882).
Comment on lines
+13484
to
+13496
| if ( $opt{'yaml'} ) { | ||
| my $yaml_str = _to_yaml( \%result ); | ||
| print $yaml_str; | ||
|
|
||
| if ( $opt{'outputfile'} ) { | ||
| unlink $opt{'outputfile'} if ( -e $opt{'outputfile'} ); | ||
| open my $fh, q(>), $opt{'outputfile'} | ||
| or die | ||
| "Unable to open $opt{'outputfile'} in write mode. please check permissions for this file or directory"; | ||
| print $fh $yaml_str; | ||
| close $fh; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scrub credentials before dumping YAML.
_to_yaml(\%result) includes MySQLTuner.options and MySQL Client.Authentication Info; those can contain pass, password, ssh-password, and -p'...'. That turns --yaml into a surprisingly efficient password printer. Please serialize a sanitized copy for machine-readable output.
🛡️ Proposed direction
+sub _sanitized_result_for_export {
+ my %copy = %{ $_[0] };
+
+ if ( ref $copy{'MySQLTuner'}{'options'} eq 'HASH' ) {
+ my %opts = %{ $copy{'MySQLTuner'}{'options'} };
+ for my $secret (qw(pass password ssh-password passenv userenv)) {
+ $opts{$secret} = '[REDACTED]' if defined $opts{$secret};
+ }
+ $copy{'MySQLTuner'}{'options'} = \%opts;
+ }
+
+ if ( defined $copy{'MySQL Client'}{'Authentication Info'} ) {
+ $copy{'MySQL Client'}{'Authentication Info'} =~ s/-p'[^']*'/-p'[REDACTED]'/g;
+ $copy{'MySQL Client'}{'Authentication Info'} =~ s/-p\S+/-p[REDACTED]/g;
+ }
+
+ return \%copy;
+}
+
if ( $opt{'yaml'} ) {
- my $yaml_str = _to_yaml( \%result );
+ my $yaml_str = _to_yaml( _sanitized_result_for_export( \%result ) );As per path instructions, mysqltuner.pl is security-sensitive for credential handling and says to “Flag any credential exposure.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysqltuner.pl` around lines 13484 - 13496, The YAML output feature exposes
sensitive credentials because _to_yaml is serializing the entire %result hash
which contains unfiltered MySQLTuner.options and MySQL Client.Authentication
Info containing passwords and SSH credentials. Before calling _to_yaml, create a
deep copy of the %result hash and remove or redact all sensitive fields that
contain credentials (such as keys matching pass, password, ssh-password, and -p
patterns), then pass this sanitized copy to _to_yaml instead of the original
%result hash. Apply this same sanitized copy to both the console output via
print and the file output via the filehandle to ensure credentials are not
leaked in either location.
Source: Path instructions
Comment on lines
+13797
to
13799
| dump_csv_files; # dump csv files | ||
| make_recommendations; # Make recommendations based on stats | ||
| dump_result; # Dump result if debug is on |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Flush buffered output when opening dumpdir raw output late.
With dump_csv_files() now called after the report sections, raw_mysqltuner.txt starts recording only from this point onward when --dumpdir is used without --outputfile. The code promises “complete analysis output”; right now it’s more of a director’s cut missing the movie.
📝 Proposed fix
if ( !$opt{outputfile} ) {
$opt{outputfile} = $raw_output_file;
my $outputfile_path = abs_path( $opt{outputfile} );
open( $fh, '>', $outputfile_path )
or die("Failed to open $outputfile_path for writing: $!");
$opt{nocolor} = 1; # Disable colors in file output
+ if (`@raw_output_lines`) {
+ print $fh join( "\n", `@raw_output_lines` ), "\n";
+ }
}Also applies to: 13540-13547
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mysqltuner.pl` around lines 13797 - 13799, Buffered output is not being
flushed before dump_csv_files() is called, causing raw_mysqltuner.txt to miss
the analysis output generated before this point when using --dumpdir without
--outputfile. Add a flush operation immediately before the dump_csv_files()
function call to ensure all buffered output is written to the raw output file,
guaranteeing the complete analysis output is captured in raw_mysqltuner.txt.
Comment on lines
+22
to
+46
| my $mock_old_file = 'mock_old.json'; | ||
| open(my $fh, '>', $mock_old_file) or die "Could not open $mock_old_file: $!"; | ||
| print $fh q({ | ||
| "General": { "Date": "2026-06-01" }, | ||
| "Stats": { | ||
| "QPS": 10.0, | ||
| "Total Data Size": 83886080 | ||
| }, | ||
| "HealthScore": 80, | ||
| "SectionalHealthScore": { | ||
| "General": 95, | ||
| "Storage": 70, | ||
| "Security": 90, | ||
| "Replication": 100, | ||
| "Modeling": 90 | ||
| } | ||
| }); | ||
| close($fh); | ||
|
|
||
| # Ensure cleanup on exit | ||
| END { | ||
| if (-e 'mock_old.json') { | ||
| unlink('mock_old.json'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use a unique temp file; shared mock_old.json is a race condition wearing a fake mustache.
Line 22 hardcodes a global filename, so parallel test runs can stomp each other’s fixture and produce flaky results.
Suggested patch
+use File::Temp qw(tempfile);
@@
-my $mock_old_file = 'mock_old.json';
-open(my $fh, '>', $mock_old_file) or die "Could not open $mock_old_file: $!";
+my ($fh, $mock_old_file) = tempfile('mock_old_XXXX', SUFFIX => '.json', UNLINK => 1);
print $fh q({
@@
close($fh);
-
-# Ensure cleanup on exit
-END {
- if (-e 'mock_old.json') {
- unlink('mock_old.json');
- }
-}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit_phase13_kpis.t` around lines 22 - 46, The hardcoded filename
'mock_old.json' in the variable $mock_old_file can cause race conditions when
tests run in parallel, as multiple test instances may read/write to the same
file simultaneously. Replace the hardcoded string with a unique temporary
filename using Perl's File::Temp module (such as tempfile() or similar) to
ensure each test gets an isolated fixture file. Update both the file creation
assignment to $mock_old_file and the cleanup check in the END block to use the
dynamically generated temporary filename instead of the string literal
'mock_old.json'.
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
…enforce changelog staging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release Notes - v2.9.0
Date: 2026-06-15
📝 Executive Summary
📈 Diagnostic Growth Indicators
🛠️ Internal Commit History
⚙️ Technical Evolutions
➕ CLI Options Added
--innodb_buffer_stats_by_schema--innodb_buffer_stats_by_table--processlist--schema_auto_increment_columns--schema_index_statistics--schema_object_overview--schema_redundant_indexes--schema_table_lock_waits--schema_table_statistics--schema_table_statistics_with_buffer--schema_tables_with_full_table_scans--schema_unused_indexes--session--statement_analysis--statements_with_errors_or_warnings--statements_with_full_table_scans--statements_with_runtimes_in_95th_percentile--statements_with_sorting--statements_with_temp_tables➖ CLI Options Deprecated
--data--template✅ Laboratory Verification Results