Project

General

Profile

Actions

Bug #16936

closed

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

Added by jaruga (Jun Aruga) almost 4 years ago. Updated over 2 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]
[ruby-core:98663]

Description

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

Related issues 1 (0 open1 closed)

Is duplicate of Ruby master - Bug #16935: Syntax error with `make check TESTS="-n /Foo#method/"`ClosedActions
Actions #1

Updated by jaruga (Jun Aruga) almost 4 years ago

  • 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
Actions #2

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

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

Updated by nobu (Nobuyoshi Nakada) almost 4 years ago

  • Status changed from Open to Closed

Applied in changeset git|0c00a4176ba353d59d8c991428574ef2c2676674.


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

Updated by jaruga (Jun Aruga) almost 4 years ago

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
...

Updated by jaruga (Jun Aruga) almost 4 years ago

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.

Updated by jaruga (Jun Aruga) almost 4 years ago

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.

Updated by jaruga (Jun Aruga) almost 4 years ago

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

Updated by jaruga (Jun Aruga) almost 4 years ago

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.

Updated by vo.x (Vit Ondruch) almost 4 years ago

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.

Updated by jaruga (Jun Aruga) almost 4 years ago

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.

Updated by jaruga (Jun Aruga) almost 4 years ago

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

Updated by jaruga (Jun Aruga) almost 4 years ago

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

Updated by vo.x (Vit Ondruch) over 2 years ago

  • 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

Actions #14

Updated by Anonymous over 2 years ago

  • 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].

Updated by vo.x (Vit Ondruch) over 2 years ago

  • 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

Updated by nagachika (Tomoyuki Chikanaga) over 2 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0