Skip to content

Backport(v1.16) test: fix Sigdump tests failure (#4846) #4850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 5, 2025
Merged

Conversation

daipom
Copy link
Contributor

@daipom daipom commented Mar 5, 2025

Which issue(s) this PR fixes:

What this PR does / why we need it:
Sigdump.dump iterates ObjectSpace.
If there are DoubleDefinition objects of RR, it can set some expectations of RR unexpectedly.
It can cause test failure.

By doing GC, we can remove DoubleDefinition objects made in other tests.

See #4063

Failure: test_cont_in_main_process_signal_handlers(SupervisorTest):
  class()
  Called 0 times.
  Expected 1 times.

(...)/sigdump-0.2.5/lib/sigdump.rb:76:in block in dump_object_count' (...)/sigdump-0.2.5/lib/sigdump.rb:75:in each_object'
(...)/sigdump-0.2.5/lib/sigdump.rb:75:in dump_object_count' (...)/sigdump-0.2.5/lib/sigdump.rb:18:in block in dump'
(...)/sigdump-0.2.5/lib/sigdump.rb:138:in open' (...)/sigdump-0.2.5/lib/sigdump.rb:138:in _open_dump_path'
(...)/sigdump-0.2.5/lib/sigdump.rb:14:in dump' (...)/fluentd/fluentd/lib/fluent/supervisor.rb:1097:in dump_non_windows'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:997:in block in install_main_process_signal_handlers' (...)/fluentd/fluentd/test/test_supervisor.rb:270:in kill'
(...)/fluentd/fluentd/test/test_supervisor.rb:270:in `test_cont_in_main_process_signal_handlers'
267: sv = Fluent::Supervisor.new({})
268: sv.send(:install_main_process_signal_handlers)
269:
=> 270: Process.kill :CONT, Process.pid
271:
272: sleep 1
273:

Docs Changes:
Not needed.

Release Note:
Not needed.

**Which issue(s) this PR fixes**:
* Fixes #4063

**What this PR does / why we need it**:
Sigdump.dump iterates ObjectSpace.
If there are DoubleDefinition objects of RR, it can set some
expectations of RR unexpectedly.
It can cause test failure.

By doing GC, we can remove DoubleDefinition objects made in other tests.

See #4063

    Failure: test_cont_in_main_process_signal_handlers(SupervisorTest):
      class()
      Called 0 times.
      Expected 1 times.
(...)/sigdump-0.2.5/lib/sigdump.rb:76:in `block in dump_object_count'
    (...)/sigdump-0.2.5/lib/sigdump.rb:75:in `each_object'
    (...)/sigdump-0.2.5/lib/sigdump.rb:75:in `dump_object_count'
    (...)/sigdump-0.2.5/lib/sigdump.rb:18:in `block in dump'
    (...)/sigdump-0.2.5/lib/sigdump.rb:138:in `open'
    (...)/sigdump-0.2.5/lib/sigdump.rb:138:in `_open_dump_path'
    (...)/sigdump-0.2.5/lib/sigdump.rb:14:in `dump'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:1097:in
`dump_non_windows'
(...)/fluentd/fluentd/lib/fluent/supervisor.rb:997:in `block in
install_main_process_signal_handlers'
    (...)/fluentd/fluentd/test/test_supervisor.rb:270:in `kill'
(...)/fluentd/fluentd/test/test_supervisor.rb:270:in
`test_cont_in_main_process_signal_handlers'
         267:     sv = Fluent::Supervisor.new({})
         268:     sv.send(:install_main_process_signal_handlers)
         269:
      => 270:     Process.kill :CONT, Process.pid
         271:
         272:     sleep 1
         273:

**Docs Changes**:
Not needed.

**Release Note**:
Not needed.

Signed-off-by: Daijiro Fukuda <fukuda@clear-code.com>
@daipom daipom added the CI Test/CI issues label Mar 5, 2025
@daipom daipom added this to the v1.16.8 milestone Mar 5, 2025
Copy link
Contributor

@kenhys kenhys left a comment

Choose a reason for hiding this comment

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

LGTM.

@kenhys kenhys merged commit 2caaec9 into v1.16 Mar 5, 2025
15 of 16 checks passed
@kenhys kenhys deleted the backport-4846 branch March 5, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Test/CI issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants