Skip to content

testcase: remove needless instance variables added during test runs #303

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.

Sign up for GitHub

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

Jump to bottom
Merged
kou merged 1 commit into from
Mar 19, 2025
Merged

testcase: remove needless instance variables added during test runs #303

kou merged 1 commit into from
Mar 19, 2025

Conversation

@tikkss
Copy link
Contributor

@tikkss tikkss commented Mar 14, 2025 •
edited by kou
Loading

GitHub: GH-235

Since c26354f, instance variables add during test runs persisted longer than before. This patch ensures they are garbage collected at any time after each test run.

Reported by akira yamada. Thanks!!!

@tikkss @kou
testcase: remove needless instance variables added during test runs
f5c0301 
GitHub: test-unitGH-235

Since c26354f, instance variables add during test runs persisted longer
than before. This patch ensures they are garbage collected at any time
after each test run.

Co-authored-by: Sutou Kouhei <kou@clear-code.com>
@kou
Copy link
Member

kou commented Mar 15, 2025

Does the JRuby CI failure relate to this change?

https://github.com/test-unit/test-unit/actions/runs/13867125431/job/38808320158?pr=303#step:5:14

===============================================================================
Error: test_notify(TestUnitNotification): NoMethodError: undefined method 'runner_class' for nil
/home/runner/work/test-unit/test-unit/test/testunit-test-util.rb:29:in '_run_test'
/home/runner/work/test-unit/test-unit/test/test-notification.rb:31:in '_run_test'
/home/runner/work/test-unit/test-unit/test/test-notification.rb:21:in 'test_notify'
     18:   end
     19: 
     20:   def test_notify
  => 21:     result = _run_test("test_notify")
     22:     assert_equal("1 tests, 0 assertions, 0 failures, 0 errors, 0 pendings, " \
     23:                  "0 omissions, 2 notifications",
     24:                  result.to_s)
org/jruby/RubyKernel.java:1382:in 'catch'
org/jruby/RubyKernel.java:1377:in 'catch'
org/jruby/RubyArray.java:2076:in 'each'
org/jruby/RubyArray.java:2076:in 'each'
org/jruby/RubyKernel.java:1382:in 'catch'
org/jruby/RubyKernel.java:1377:in 'catch'
Error: NoMethodError: undefined method 'runner_class' for nil
/home/runner/work/test-unit/test-unit/test/testunit-test-util.rb:29:in '_run_test'
/home/runner/work/test-unit/test-unit/test/test-notification.rb:31:in '_run_test'
/home/runner/work/test-unit/test-unit/test/test-notification.rb:21:in 'test_notify'
org/jruby/RubyKernel.java:1382:in 'catch'
org/jruby/RubyKernel.java:1377:in 'catch'
org/jruby/RubyArray.java:2076:in 'each'
org/jruby/RubyArray.java:2076:in 'each'
org/jruby/RubyKernel.java:1382:in 'catch'
org/jruby/RubyKernel.java:1377:in 'catch'
===============================================================================

@kou
Copy link
Member

kou commented Mar 15, 2025

https://github.com/test-unit/test-unit/actions/runs/13870167318/job/38815791782#step:5:12

It seems that this is a JRuby problem.

@tikkss
Copy link
Contributor Author

tikkss commented Mar 16, 2025

Thanks for your investigate. I also think that this is a JRuby problem.

Recently, jruby-head switches to JRuby 10 from 9.4.
It seems that Method#arity does not handle keyword arguments correctly in JRuby 10.

test-unit/lib/test/unit/test-suite-runner.rb

Lines 95 to 101 in 22a36aa

if test.method(:run).arity == -2
test.run(result, run_context: run_context, &event_listener)
else
# For backward compatibility. There are scripts that overrides
# Test::Unit::TestCase#run without keyword arguments.
test.run(result, &event_listener)
end

test-unit/lib/test/unit/testsuite.rb

Line 50 in 22a36aa

def run(result, run_context: nil, &progress_block)

Steps to reproduce:

$ jruby -v -e '
class TestSuite
  def run(result, run_context: nil, &progress_block)
  end
end
c = TestSuite.new
p c.method(:run).arity
'

Expected result:

jruby 10.0.0.0-SNAPSHOT (3.4.2) 2025-03-15 a4e4d7dc4c OpenJDK 64-Bit Server VM 21+35-2513 on 21+35-2513 +indy +jit [x86_64-linux]
-2

Actual result:

jruby 10.0.0.0-SNAPSHOT (3.4.2) 2025-03-15 a4e4d7dc4c OpenJDK 64-Bit Server VM 21+35-2513 on 21+35-2513 +indy +jit [x86_64-linux]
1

I reported this issue to JRuby in jruby/jruby#8698.

See also:
jruby/jruby#8675

@tikkss tikkss mentioned this pull request Mar 18, 2025
Open
@kou kou merged commit 8bd2721 into test-unit:master Mar 19, 2025
31 of 32 checks passed
@kou
Copy link
Member

kou commented Mar 19, 2025

Thanks!

@tikkss tikkss deleted the testcase-remove-needless-ivar branch March 19, 2025 21:03
@tikkss
Copy link
Contributor Author

tikkss commented Mar 19, 2025

Thanks for your review!

JRuby problem has been resolved by jruby/jruby#8706.
So JRuby 10 also returns the expected result:

$ jruby -v -e '
class C
  def x(a, b: nil, &c)
  end
end
c = C.new
p c.method(:x).arity
'
jruby 10.0.0.0-SNAPSHOT (3.4.2) 2025-03-19 69be2d1891 OpenJDK 64-Bit Server VM 21+35-2513 on 21+35-2513 +indy +jit [x86_64-linux]
-2

The JRuby CI failure may turn into success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tikkss @kou