search

palkan / active_event_store

Rails Event Store in a more Rails way
MIT License
182 stars 13 forks source link

Implement assertions for Minitest #2

Closed chriscz closed 3 years ago

chriscz commented 4 years ago

What is the purpose of this pull request?

This pull request tracks an implementation of some minitest assertions & expectations.

What changes did you make? (overview)

So far I've added the ActiveEventStore::Minitest::Assertions module and the assert_published method and I plan on adding an assert_enqueued_async_subscriber method as well.

Is there anything you'd like reviewers to focus on?

Nothing for review, but some questions:

  1. How would you prefer I test these assertions?
  2. Any convention or templates from other Evil Martian projects that I should follow here?

Checklist

palkan commented 4 years ago

Thanks for you contribution!

How would you prefer I test these assertions?

Check out, for example: https://github.com/palkan/action_policy/blob/master/test/action_policy/test_helper_test.rb

Any convention or templates from other Evil Martian projects that I should follow here?

Let's define assertions in the ActiveEventStore::TestHelper file—that's a common practice used in Rails itself and other libraries.

I've implemented assert_published

What about a less generic name? Say, assert_event_published (similarly to RSpec have_published_event).

chriscz commented 4 years ago

Hi @palkan I've made some of the changes you suggested. I can't help but notice the similarity between the rspec and minitest matchers and I'm wondering if a refactor would make sense?

When I run the tests using rake test I noticed that my import of bundler prints out a lot of deprecation notices and the test run seems very noisy. I can't seem to go without importing bundler as I get an error if I don't. I don't have much time to investigate tonight, but here's an example of the output so long:

WARN: Unresolved specs during Gem::Specification.reset:
      minitest (~> 5.1)
      rails-html-sanitizer (>= 1.2.0, ~> 1.0, ~> 1.1)
      nokogiri (>= 1.6)
      builder (~> 3.1)
      erubi (~> 1.4)
      rake (>= 0.8.7)
      method_source (>= 0)
WARN: Clearing out unresolved specs.
Please report a bug if this causes problems.
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/version.rb:4: warning: already initialized constant JSON::VERSION
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/version.rb:4: warning: previous definition of VERSION was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/version.rb:5: warning: already initialized constant JSON::VERSION_ARRAY
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/version.rb:5: warning: previous definition of VERSION_ARRAY was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/version.rb:6: warning: already initialized constant JSON::VERSION_MAJOR
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/version.rb:6: warning: previous definition of VERSION_MAJOR was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/version.rb:7: warning: already initialized constant JSON::VERSION_MINOR
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/version.rb:7: warning: previous definition of VERSION_MINOR was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/version.rb:8: warning: already initialized constant JSON::VERSION_BUILD
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/version.rb:8: warning: previous definition of VERSION_BUILD was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/common.rb:100: warning: already initialized constant JSON::NaN
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/common.rb:107: warning: previous definition of NaN was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/common.rb:102: warning: already initialized constant JSON::Infinity
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/common.rb:109: warning: previous definition of Infinity was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/common.rb:104: warning: already initialized constant JSON::MinusInfinity
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/common.rb:111: warning: previous definition of MinusInfinity was here
/home/chris/.rbenv/versions/2.5.8/lib/ruby/2.5.0/json/common.rb:129: warning: already initialized constant JSON::UnparserError
/home/chris/.rbenv/versions/2.5.8/lib/ruby/gems/2.5.0/gems/json-2.3.1/lib/json/common.rb:136: warning: previous definition of UnparserError was here
      create  db/migrate/20200929192123_create_event_store_events.rb
Run options: --seed 24960

# Running:

.......

Finished in 0.110304s, 63.4607 runs/s, 81.5924 assertions/s.
7 runs, 9 assertions, 0 failures, 0 errors, 0 skips
chriscz commented 4 years ago

I got the output cleared by running bundle clean --force so it looks like an issue with my local gems, also llooks like I forgot to run with bundle exec :facepalm:

mostlyobvious commented 3 years ago

:wave:

For the record — there's an interest for minitest matchers in upstream too:

Perhaps we can team up on this?

chriscz commented 3 years ago

Hey @pawelpacana sure thing! In a recent project we've been using some helpers which might be of use in RES: https://gist.github.com/chriscz/14a8b5c9874706a9d8c07fe50041f2b1

palkan commented 3 years ago

Hey @chriscz! Sorry, are you waiting on some feedback from my side? Or we're ready to merge it?

(Probably, we should wait for #6 first)

chriscz commented 3 years ago

Hey @palkan I'm rounding this off tonight will mark ready for review then. Just need to update the docs and I'll be done.

palkan commented 3 years ago

Released in 1.0.1.

Thank you!