Skip to content
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

assert_changes works on including ActiveSupport::Assertions class #42870

Merged

Conversation

pedrosnk
Copy link
Contributor

Summary

Hi beloved rails team. This is my attempt to fix #42869.

On the two possible approaches that I proposed on the issue, moving the alias to the Assertions module doesn't work because the alias assignment happens on loading the class, which fails because the method that the alias will redirect to doesn't exist yet. So I just renamed the method to use the original one from rails that

assert_not_equal is an alias for refute_equal that is defined only on the class ActiveSupport::TestCase. This commit ensures ActiveSupport::Testing::Assertions#assert_changes doesn't depends on ActiveSupport::TestCase to work.

Other Information

As I said on the issue I created, this used to work on 6.0.4 and on previously rails versions, stopped worked after we upgraded to rails 6.1

The code that I'm running started to fail on rails 6.1 is something like

require "minitest/autorun"
require "active_support/testing/assertions"

describe :assert_changes do
  include ActiveSupport::Testing::Assertions

  it "counts" do
    counter = 1
    assert_changes( -> { counter } ) do
      counter = 2
    end
  end
end

assert_not_equal is an alias for refute_equal that is defined only on
the class ActiveSupport::TestCase. This commit ensures
ActiveSupport::Testing::Assertions#assert_changes doesn't depends on
ActiveSupport::TestCase to work.
@ghiculescu
Copy link
Member

This looks good, it looks like 074265a is the culprit. Are there any other methods that need fixing or just assert_changes?

@pedrosnk
Copy link
Contributor Author

This looks good, it looks like 074265a is the culprit. Are there any other methods that need fixing or just assert_changes?

The only place that uses an assert_* that is not aliased is this one as I could see.

@ghiculescu ghiculescu added the ready PRs ready to merge label Jul 27, 2021
@byroot byroot merged commit 8a22369 into rails:main Jul 28, 2021
@pedrosnk pedrosnk deleted the pm/import_of_active_support_test_assertions branch July 28, 2021 14:40
rafaelfranca pushed a commit that referenced this pull request Jul 28, 2021
…est_assertions

assert_changes works on including ActiveSupport::Assertions class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
activesupport ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assert_changes stopped working after rails 6.1 on including ActiveSupport::Testing::Assertions
3 participants