Fix unstubbing of not defined methods

[GermanDZ]

After stubbing a method not really defined, but the module (probably will behave the same for classes) accepts a message (respond to...) and it's managed by meta-prorgramming used "method_missing" then the unstubbing is not working because the stubbed method is still there.

This fix the behaviour, ensuring after the stubbing block the stubbed method is removed and calls to "respond_to..." are called as usual.

[GermanDZ]

Rebased to the latest master from upstream.

[alekseyl]

Technically your case is a subset of the #891

i.e. the method should be kept on the singleton_class only if it was defined on the singleton_class already. Otherwise it should be removed after stubbing is done.

You can examine your added test cases against my branch.

[GermanDZ]

Great! my tests are passing with your fix. 👏

I will close this PR once your PR is merged.

[alekseyl]

Cool, but we still should do something with the tests you wrote for dynamic module definitions. May be re-PR with tests only.

[GermanDZ]

👍 I will rebase this branch after your merge and use my tests to protect the code against regressions.

[luke-gru]

I think this solution is good. Question though: why, in the test, are you manually setting @assertion_count? Also, can the maintainers please take a look at this issue?

[GermanDZ]

Question though: why, in the test, are you manually setting @assertion_count?

Probably just left-overs from previous code, tomorrow I'll rebase/re-run the code.

[GermanDZ]

Rebased and removed the fix (only added the tests), still failing.

Ruby 2.7

➜  minitest git:(fix-stubbing-not-really-defined-methods) rake test
Run options: --seed 47573

# Running:

..............................................................S............................................................................S...S......S.....S..F....S..S.SSSS..................................................S..............................S...............S...................S.........................................S...........S.......................................................................................

Finished in 0.232336s, 1859.3761 runs/s, 5375.8350 assertions/s.

  1) Failure:
TestMinitestStub#test_dynamic_method_on_modules [/Users/germandz/Development/minitest/test/minitest/test_minitest_mock.rb:761]:
Expected: false
  Actual: true

432 runs, 1249 assertions, 1 failures, 0 errors, 17 skips

You have skipped tests. Run with --verbose for details.
rake aborted!
Command failed with status (1): [/Users/germandz/.rubies/ruby-2.7.1/bin/rub...]
/Users/germandz/Development/minitest/lib/minitest/test_task.rb:164:in `block in define'
/Users/germandz/.gem/ruby/2.7.1/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'
Tasks: TOP => test
(See full trace by running task with --trace)

Ruby 3.2

➜  minitest git:(fix-stubbing-not-really-defined-methods) rake test
Run options: --seed 29173

# Running:

.........................................................................................................................................................................................................................................................................................................................................................................S...S......S.....S..F....S..S.SSSS.....................................

Finished in 0.238538s, 1811.0322 runs/s, 5303.1383 assertions/s.

  1) Failure:
TestMinitestStub#test_dynamic_method_on_modules [/Users/germandz/Development/minitest/test/minitest/test_minitest_mock.rb:761]:
Expected: false
  Actual: true

432 runs, 1265 assertions, 1 failures, 0 errors, 10 skips

You have skipped tests. Run with --verbose for details.
rake aborted!
Command failed with status (1): [/Users/germandz/.rubies/ruby-3.2.2/bin/rub...]
/Users/germandz/Development/minitest/lib/minitest/test_task.rb:164:in `block in define'
Tasks: TOP => test
(See full trace by running task with --trace)

[GermanDZ]

I think this solution is good. Question though: why, in the test, are you manually setting @assertion_count? Also, can the maintainers please take a look at this issue?

I reviewed the code: @assertion_count is asserted in teardown to ensure all the expected assertions were run.

[luke-gru]

It seems the force-push got rid of your changes to the stub implementation and only kept the tests.

Also can you change the test name to be more explanatory, something like: test_respond_to_is_false_after_stub_nonexisting_method

[GermanDZ]

It seems the force-push got rid of your changes to the stub implementation and only kept the tests.

Yes, I've rebased to latest version from upstream (my branch was really old). I wanted to be sure the error is still there.

If #891 is fixing the problems I want to know if my test cases are covered.