Bug #16936: `make test-all TESTS="-n !/Foo#method/"` not skipping the test case

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: DONE

[jaruga (Jun Aruga)]

On the current latest master cf1adf985ab78507250db0e217a0fdd779e2c6e6.

$ autoconf

$ ./configure --prefix=$(pwd)/dest

$ make

The following make test-all works as I mentioned at #16935.

$ make test-all TESTS="-v -n /\^TestBugReporter#test_bug_reporter_add\$$/ -n /\^TestProcess#test_status_quit\$$/"
...
[1/0] TestBugReporter#test_bug_reporter_add = 0.41 s
[2/0] TestProcess#test_status_quit = 0.35 s
Finished tests in 9.392046s, 0.2129 tests/s, 1.5971 assertions/s.
2 tests, 15 assertions, 0 failures, 0 errors, 0 skips
...

But it seems that the following TestBugReporter#test_bug_reporter_add and TestProcess#test_status_quit are not actually skipped.
Is it a bug?

$ make test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/\^TestBugReporter#test_bug_reporter_add\$$/ -n \!/\^TestProcess#test_status_quit\$$/" 2>&1 | tee make.log
Run options:-
  --seed=21367
  "--ruby=./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems"
  --excludes-dir=./test/excludes
  --name=!/memory_leak/
  -v  
  -n  
  "!/^TestBugReporter#test_bug_reporter_add$/"
  -n  
  "!/^TestProcess#test_status_quit$/"

# Running tests:

[  1/142] TestBugReporter#test_bug_reporter_add = 0.49 s
...
[128/142] TestProcess#test_status_quit = 0.30 s
...
Finished tests in 25.978847s, 5.4660 tests/s, 39.2627 assertions/s.
142 tests, 1020 assertions, 0 failures, 0 errors, 0 skips

I want to skip the specific test cases by make test-all TESTS="..." like this. I want to specify both the testing class (ex. TestBugReporter) and method (ex. test_bug_reporter_add) with the regular expression perfect matching to avoid unintended test cases are skipped.

$ make test-all TESTS="-v -n \!/\^TestBugReporter#test_bug_reporter_add\$$/ -n \!/\^TestProcess#test_status_quit\$$/"

The following make test works skipping test_bug_reporter_add and test_status_quit methods.

$ make test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/test_bug_reporter_add\$$/ -n \!/test_status_quit\$$/" 2>&1 | tee make2.log
...
140 tests, 1005 assertions, 0 failures, 0 errors, 0 skips

[jaruga (Jun Aruga)]

• Subject changed from `make check TESTS="-n !/Foo#method/"` not skipping the test case to `make test-all TESTS="-n !/Foo#method/"` not skipping the test case

[nobu (Nobuyoshi Nakada)]

• Is duplicate of Bug #16935: Syntax error with `make check TESTS="-n /Foo#method/"` added

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset git|0c00a4176ba353d59d8c991428574ef2c2676674.

---

Hash marks in Makefile need to be escaped [Bug #16935]

[jaruga (Jun Aruga)]

Hash marks in Makefile need to be escaped

Thanks for the answer. In this case of the ticket, when running with the escaped hash \#, the tests are not skipped. Is this way wrong?

$ make V=1 test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/\^TestBugReporter\#test_bug_reporter_add\$$/ -n \!/\^TestProcess\#test_status_quit\$$/" 2>&1 | tee make.log

exec ./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems "./test/runner.rb" --ruby="./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems" --excludes-dir=./test/excludes --name=!/memory_leak/  test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/\^TestBugReporter\#test_bug_reporter_add$/ -n \!/\^TestProcess\#test_status_quit$/
...
Run options: 
  --seed=66754
  "--ruby=./miniruby -I./lib -I. -I.ext/common  ./tool/runruby.rb --extout=.ext  -- --disable-gems"
  --excludes-dir=./test/excludes
  --name=!/memory_leak/
  -v
  -n
  "!/^TestBugReporter#test_bug_reporter_add$/"
  -n
  "!/^TestProcess#test_status_quit$/"

# Running tests:

[  1/142] TestBugReporter#test_bug_reporter_add = 0.42 s
...
[128/142] TestProcess#test_status_quit = 0.33 s
...

I am reading and debugging the source code tool/lib/test/unit.rb filter logic, seeing the following command result.

$ ruby tool/test/runner.rb --help
...
    -n, --name PATTERN               Filter test method names on pattern: /REGEXP/, !/REGEXP/ or STRING
...

[jaruga (Jun Aruga)]

Could you show me the actual command based on the following one that was able to skip the tests on your local environment?

$ make V=1 test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/\^TestBugReporter\#test_bug_reporter_add\$$/ -n \!/\^TestProcess\#test_status_quit\$$/" 2>&1 | tee make.log

Thanks.

[jaruga (Jun Aruga)]

https://github.com/ruby/ruby/blob/465b5dc124917b828a5964c50c4e470a0c03dcf4/tool/lib/test/unit.rb#L742

The following part matches only type (= test_methods) of the suites (Array of testing class such as TestBugReporter) It seems the testing class in filter is not considered.

        total = if filter
                  suites.inject(0) {|n, suite| n + suite.send(type).grep(filter).size}
                else
                  suites.inject(0) {|n, suite| n + suite.send(type).size}
                end

However if it matches only testing method, why the following command with the positive PATTERN matches the 2 test methods as expected? Actually I think 0 in [1/0] (tool/lib/test/unit.rb#_prepare_run total = 0) is wrong number in the case. That should be 2.

$ make test-all TESTS="-v -n /\^TestBugReporter#test_bug_reporter_add\$$/ -n /\^TestProcess#test_status_quit\$$/"
...
[1/0] TestBugReporter#test_bug_reporter_add = 0.41 s
[2/0] TestProcess#test_status_quit = 0.35 s
Finished tests in 9.392046s, 0.2129 tests/s, 1.5971 assertions/s.
2 tests, 15 assertions, 0 failures, 0 errors, 0 skips
...

Why the 2 tests are executed?
Possibly when printing and running the test, the TestClass#test_method is considered in the following part.

https://github.com/ruby/ruby/blob/465b5dc124917b828a5964c50c4e470a0c03dcf4/tool/lib/test/unit.rb#L833

        def print(s)
          case s
          when /\A(.*\#.*) = \z/ # <= Testing class is considered to run it.
            runner.new_test($1)

I want to see the testing class is supported for the pattern matching.

[jaruga (Jun Aruga)]

It seems there is an issue [142/140] when using \^ in the pattern.

$ make V=1 test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/\^test_bug_reporter_add\$$/ -n \!/\^test_status_quit\$$/" 2>&1 | tee make.log
...
[  1/140] TestBugReporter#test_bug_reporter_add = 0.44 s
...
[128/140] TestProcess#test_status_quit = 0.31 s
...
[142/140] TestProcess#test_waitall = 0.18 s
Finished tests in 25.888517s, 5.4851 tests/s, 39.3997 assertions/s.
142 tests, 1020 assertions, 0 failures, 0 errors, 0 skips

Here is okay case without using \^.

$ make V=1 test-all TESTS="test/-ext-/bug_reporter/test_bug_reporter.rb test/ruby/test_process.rb -v -n \!/test_bug_reporter_add\$$/ -n \!/test_status_quit\$$/" 2>&1 | tee make.log
...
[140/140] TestProcess#test_waitall = 0.18 s
Finished tests in 25.745323s, 5.4379 tests/s, 39.0362 assertions/s.
140 tests, 1005 assertions, 0 failures, 0 errors, 0 skips

[jaruga (Jun Aruga)]

Though I do not know how Ruby project's test/unit has been implemented.
I am asking test-unit project about the how to ignore only a test name in the specific test suite.
https://github.com/test-unit/test-unit/issues/169

I would like to align the way with the test-unit project if we improve something for test/unit in Ruby project.

[vo.x (Vit Ondruch)]

test-unit in Ruby is thin wrapper above old version of minitest and does not have too much in common with test-unit project. Changing this would be quite big feat.

[jaruga (Jun Aruga)]

test-unit in Ruby is thin wrapper above old version of minitest and does not have too much in common with test-unit project. Changing this would be quite big feat.

Thanks for the info.

I did not intend to implement the new features of test/unit in Ruby project such as new command options --ignore-foo used in the test-unit project.
My intent was to look for the best direction or way to ignore only a test name in the specific test suite in test/unit of Ruby.

[jaruga (Jun Aruga)]

Seeing the current minitest and unit-test updated by the above ticket, it seems the both implementation supports "TestCase#name" perfect matching only if filter is String.

https://github.com/seattlerb/minitest/blob/32d49db55d80b8479237898f07d161bb52ef905c/lib/minitest.rb#L317-L322

      filter = options[:filter] || "/./"
      filter = Regexp.new $1 if filter.is_a?(String) && filter =~ %r%/(.*)/%

      filtered_methods = self.runnable_methods.find_all { |m|
        filter === m || filter === "#{self}##{m}"
      }

https://github.com/test-unit/test-unit/blob/74f2ddce2fe7c54bde281082acbfd90c0c953485/lib/test/unit/autorunner.rb#L521-L529

      def match_test_name(test, pattern)
        return true if pattern === test.method_name
        return true if pattern === test.local_name
        if pattern.is_a?(String)
          return true if pattern === "#{test.class}##{test.method_name}"
          return true if pattern === "#{test.class}##{test.local_name}"
        end
        false
      end

[jaruga (Jun Aruga)]

I noticed the test/unit negative filter was implemented as Ruby project's original code in 2016.
https://github.com/ruby/ruby/commit/83e36bb5a6e02b747d10c1baf5e1b7ff2b4c49fe

[vo.x (Vit Ondruch)]

• Status changed from Closed to Open
• ruby -v set to ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]

This was somehow swept under the carpet, but it is still unresolved. I have opened PR with a fix:

https://github.com/ruby/ruby/pull/5026

• Status changed from Open to Closed

Applied in changeset git|5086c25f6015558877f85c3f1c014780b08fd3ce.

---

Properly exclude test cases.

Lets consider the following scenario:

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):001:0> p suite
OpenSSL::TestEC
=> OpenSSL::TestEC

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):002:0> p all_test_methods
["test_ECPrivateKey", "test_ECPrivateKey_encrypted", "test_PUBKEY", "test_check_key", "test_derive_key", "test_dh_compute_key", "test_dsa_sign_asn1_FIPS186_3", "test_ec_group", "test_ec_key", "test_ec_point", "test_ec_point_add", "test_ec_point_mul", "test_generate", "test_marshal", "test_sign_verify", "test_sign_verify_raw"]
=>
["test_ECPrivateKey",
 "test_ECPrivateKey_encrypted",
 "test_PUBKEY",
 "test_check_key",
 "test_derive_key",
 "test_dh_compute_key",
 "test_dsa_sign_asn1_FIPS186_3",
 "test_ec_group",
 "test_ec_key",
 "test_ec_point",
 "test_ec_point_add",
 "test_ec_point_mul",
 "test_generate",
 "test_marshal",
 "test_sign_verify",
 "test_sign_verify_raw"]

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):003:0> p filter
/\A(?=.*)(?!.*(?-mix:(?-mix:memory_leak)|(?-mix:OpenSSL::TestEC.test_check_key)))/
=> /\A(?=.*)(?!.*(?-mix:(?-mix:memory_leak)|(?-mix:OpenSSL::TestEC.test_check_key)))/

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):004:0> method = "test_check_key"
=> "test_check_key"

The intention here is to exclude the test_check_key test case.
Unfortunately this does not work as expected, because the negative filter
is never checked:

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):005:0> filter === method
=> true

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):006:0> filter === "#{suite}##{method}"
=> false

irb(#<Test::Unit::AutoRunner::Runner:0x0000560f68afc3c8>):007:0> filter === method || filter === "#{suite}##{method}"
=> true

Therefore always filter against the fully qualified method name
#{suite}##{method}, which should provide the expected result.

However, if plain string filter is used, keep checking also only the
method name.

This resolves [Bug #16936].

[vo.x (Vit Ondruch)]

• Backport changed from 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN to 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: REQUIRED

Setting the backport flag, because it is useful to disable some test cases in Fedora package, if needed (and it will be included there soonish). But it is not super important. So feel free to ignore this backport request. Thx

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: REQUIRED to 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: DONE

ruby_3_0 1d29740c1b101db4bd8fc2d05f929a9e37471a0f merged revision(s) 5086c25f6015558877f85c3f1c014780b08fd3ce,3ff0a0b40c2e1fbdad2286f1dafe837f822d0e0d.