Skip to content

[UT] test isolation workgroup manager - #67489

Merged
dirtysalt merged 1 commit into
StarRocks:mainfrom
alifcapital:fix/test-isolation-workgroup-manager
May 15, 2026
Merged

[UT] test isolation workgroup manager#67489
dirtysalt merged 1 commit into
StarRocks:mainfrom
alifcapital:fix/test-isolation-workgroup-manager

Conversation

@eshishki

@eshishki eshishki commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix test isolation issues introduced in #66690 and #63956.

When tests run in the same process (without gtest-parallel),
ResourceGroupUsageRecorderTest replaces workgroup_manager without calling
start(), leaving driver_executor as nullptr. This causes subsequent tests
to crash with nullptr dereference.

Changes

  • ResourceGroupUsageRecorderTest: save/restore original workgroup_manager, call start()
  • PipeLineFileScanNodeTest: add missing set_workgroup() call

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5
    • 3.4
    • 3.3

Note

Ensures test isolation and proper workgroup setup to avoid nullptr crashes across tests.

  • ResourceGroupUsageRecorderTest: replace exec_env._workgroup_manager with a started instance, then restore the original at the end
  • PipeLineFileScanNodeTest: call _fragment_ctx->set_workgroup(ExecEnv::GetInstance()->workgroup_manager()->get_default_workgroup()) in SetUp()

Written by Cursor Bugbot for commit 0545875. This will update automatically on new commits. Configure here.

@eshishki
eshishki force-pushed the fix/test-isolation-workgroup-manager branch from ffebe53 to 0545875 Compare January 5, 2026 14:54
@alvin-phoenix-ai

Copy link
Copy Markdown
Contributor

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!

@eshishki
eshishki force-pushed the fix/test-isolation-workgroup-manager branch 2 times, most recently from 700c918 to 0158661 Compare May 13, 2026 11:21
ResourceGroupUsageRecorderTest was replacing the global workgroup_manager
without restoring the original, leaving subsequent tests with a manager
they did not own. This caused PipeLineFileScanNodeTest and others to crash
when run in the same process without gtest-parallel isolation.

Changes:
- Save and restore original workgroup_manager in ResourceGroupUsageRecorderTest
- Add set_workgroup() call in PipeLineFileScanNodeTest setup

Signed-off-by: Evgeniy Shishkin <eshishki@gmail.com>
@eshishki
eshishki force-pushed the fix/test-isolation-workgroup-manager branch from 0158661 to 9b09710 Compare May 13, 2026 12:11
@eshishki eshishki closed this May 13, 2026
@eshishki
eshishki deleted the fix/test-isolation-workgroup-manager branch May 13, 2026 17:08
@eshishki
eshishki restored the fix/test-isolation-workgroup-manager branch May 13, 2026 17:09
@eshishki eshishki reopened this May 13, 2026
@github-actions github-actions Bot added the 4.1 label May 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

[Java-Extensions Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions

Copy link
Copy Markdown
Contributor

[FE Incremental Coverage Report]

pass : 0 / 0 (0%)

@github-actions

Copy link
Copy Markdown
Contributor

[BE Incremental Coverage Report]

pass : 0 / 0 (0%)

@eshishki

Copy link
Copy Markdown
Contributor Author

@alvin-celerdata could you take a look when you have a moment? This is a small BE UT-only fix for test isolation in the workgroup manager.

@stdpain stdpain changed the title [BugFix] test isolation workgroup manager [UT] test isolation workgroup manager May 15, 2026
@dirtysalt
dirtysalt merged commit a58a55f into StarRocks:main May 15, 2026
128 of 141 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

@Mergifyio backport branch-4.1

@github-actions github-actions Bot removed the 4.1 label May 15, 2026
@mergify

mergify Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

backport branch-4.1

✅ Backports have been created

Details

Cherry-pick of a58a55f has failed:

On branch mergify/bp/branch-4.1/pr-67489
Your branch is up to date with 'origin/branch-4.1'.

You are currently cherry-picking commit a58a55faf7.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   be/test/agent/resource_group_usage_recorder_test.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   be/test/exec/pipeline/pipeline_file_scan_node_test.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

wanpengfei-git pushed a commit that referenced this pull request Jun 2, 2026
Signed-off-by: stdpain <34912776+stdpain@users.noreply.github.com>
Co-authored-by: Evgeniy Shishkin <eshishki@gmail.com>
Co-authored-by: stdpain <34912776+stdpain@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants