Skip to content

build: make MySQLX bridge dependencies deterministic - #6117

Merged
renecannao merged 2 commits into
v3.0from
agent/deterministic-mysqlx-test-build
Aug 19, 2026
Merged

build: make MySQLX bridge dependencies deterministic#6117
renecannao merged 2 commits into
v3.0from
agent/deterministic-mysqlx-test-build

Conversation

@renecannao

@renecannao renecannao commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • model the two top-level MySQLX bridge tests as one shared build dependency
  • delegate both binaries to a single unit submake while preserving each public symlink target
  • add a CI regression contract for the TAP Makefile dependency graph

Root cause

test_mysqlx_plugin_load-t and test_mysqlx_admin_tables-t each launched an independent recursive make in test/tap/tests/unit. The parent Makefile therefore had no dependency edge connecting their common helper objects and plugin build. During a parallel TAP build, both child makes could independently decide that unit/obj/test_init.o needed rebuilding and write the same output.

The shared parent target makes the dependency boundary explicit: one child make now owns both binaries and their common prerequisites. Parallelism remains enabled inside that child dependency graph.

Validation

  • Regression test first failed by observing two independent unit submakes.
  • After the Makefile change it observes one unit submake containing both targets and both expected symlinks.
  • Dependency contract passed 50/50 repeated runs.
  • Full CI-lint-groups-json command set passed locally.
  • python3 -m py_compile and git diff --check passed.

The PR's normal TSAN workflow provides the full Docker build verification.


Summary by cubic

Makes MySQLX bridge test builds deterministic by modeling both binaries as one shared dependency that runs a single unit submake. Previously each target invoked its own submake and could race rebuilding shared objects; now one parent target owns both while preserving per-target symlinks.

  • Introduces MYSQLX_BRIDGE_TESTS and a phony mysqlx-bridge-tests in test/tap/tests/Makefile; test_mysqlx_plugin_load-t and test_mysqlx_admin_tables-t now depend on it.
  • Keeps ln -fs unit/$@ $@ symlinks so each target remains addressable by name.
  • Adds test/tap/groups/test_makefile_dependencies.py and runs it in the CI-lint-groups-json workflow to assert one unit submake and both symlinks.
  • No test runtime changes; only the TAP Makefile dependency graph changes. Parallelism remains inside the unit submake.

Written for commit 472fd5d. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved the MySQL X Bridge test build process so related targets are built consistently and efficiently.
    • Ensured both test targets produce the expected runnable binaries.
  • Tests

    • Added automated regression coverage to verify shared build dependencies and generated test links.
    • Integrated the validation into continuous integration checks.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4422a875-117f-485e-93af-f749b33a278f

📥 Commits

Reviewing files that changed from the base of the PR and between 19ad0b8 and 0062fbe.

📒 Files selected for processing (3)
  • .github/workflows/CI-lint-groups-json.yml
  • test/tap/groups/test_makefile_dependencies.py
  • test/tap/tests/Makefile

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: Gitar
  • GitHub Check: run / trigger
  • GitHub Check: lint
  • GitHub Check: build
  • GitHub Check: lint
🧰 Additional context used
🪛 ast-grep (0.45.1)
test/tap/groups/test_makefile_dependencies.py

[error] 18-34: Command coming from incoming request
Context: subprocess.run(
[
"make",
"--no-print-directory",
"-C",
str(TAP_TESTS_DIR),
"-n",
"-j2",
"MAKE=echo",
*MYSQLX_BRIDGE_TARGETS,
],
cwd=ROOT,
text=True,
capture_output=True,
check=False,
timeout=30,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.16.1)
test/tap/groups/test_makefile_dependencies.py

[error] 19-19: subprocess call: check for execution of untrusted input

(S603)


[error] 20-29: Starting a process with a partial executable path

(S607)

🔇 Additional comments (3)
test/tap/tests/Makefile (1)

296-306: LGTM!

test/tap/groups/test_makefile_dependencies.py (1)

1-63: LGTM!

.github/workflows/CI-lint-groups-json.yml (1)

28-29: LGTM!


📝 Walkthrough

Walkthrough

The TAP Makefile now builds both MySQLX bridge targets through one shared unit submake. A regression test validates the parallel dry-run output, and CI runs this dependency check.

Changes

MySQLX TAP dependency validation

Layer / File(s) Summary
Shared MySQLX bridge build
test/tap/tests/Makefile
The two MySQLX bridge targets use one shared mysqlx-bridge-tests submake and create separate symlinks.
Dependency regression check
test/tap/groups/test_makefile_dependencies.py, .github/workflows/CI-lint-groups-json.yml
The test validates the shared submake and both symlink commands during a parallel dry run. CI runs the test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0062f

This change consolidates the MySQLX bridge test build dependency and adds a regression contract for the dependency graph; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Poem

A rabbit checked the Makefile bright,
Two bridge targets built just right.
One shared submake led the way,
Symlinks bloomed for tests today.
CI hopped in: “All paths obey!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making MySQLX bridge test dependencies deterministic.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/deterministic-mysqlx-test-build

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.

❤️ Share

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

@renecannao renecannao added the ci:asan Run the central TAP build and fan-out with AddressSanitizer. label Aug 19, 2026
@gitar-bot

gitar-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Consolidates MySQLX bridge test targets into a single shared build dependency to prevent parallel race conditions on shared objects. No issues found.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 4 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.67%. Comparing base (749c471) to head (472fd5d).
⚠️ Report is 3 commits behind head on v3.0.

Additional details and impacted files
@@            Coverage Diff             @@
##             v3.0    #6117      +/-   ##
==========================================
+ Coverage   60.80%   62.67%   +1.87%     
==========================================
  Files         613      614       +1     
  Lines      175976   178202    +2226     
  Branches    44535    44752     +217     
==========================================
+ Hits       107002   111691    +4689     
+ Misses      47514    44710    -2804     
- Partials    21460    21801     +341     
Flag Coverage Δ
integration-tests 58.97% <ø> (+0.41%) ⬆️
simulation-tests 27.22% <ø> (?)
unit-tests 17.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@renecannao
renecannao marked this pull request as ready for review August 19, 2026 16:06
@renecannao
renecannao merged commit 6f20080 into v3.0 Aug 19, 2026
86 of 91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:asan Run the central TAP build and fan-out with AddressSanitizer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant