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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code lens support for rails active support test cases using minitest/spec #258

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

beauraF
Copy link
Contributor

@beauraF beauraF commented Feb 15, 2024

Hello 馃憢

This is introducing code lens support for minitest/spec

image

@beauraF beauraF requested a review from a team as a code owner February 15, 2024 18:39
@beauraF
Copy link
Contributor Author

beauraF commented Feb 15, 2024

I have signed the CLA!

@vinistock
Copy link
Member

Thank you for the contribution! The code looks good to me, but I do have a few questions as I never used minitest/spec in combination with ActiveSupport::TestCase.

  1. Are the tests still expected to inherit from ActiveSupport::TestCase?
  2. If I'm not mistaken, Rails' recommendation is to use test classes to group related examples, but minitest/spec brings in the describe method as well. Does describe work in this case? Do we need to handle it?

@beauraF
Copy link
Contributor Author

beauraF commented Feb 20, 2024

Thanks @vinistock ! Sorry, I should have add more details ; it was planned, but haven't took the time to come back to it.

Are the tests still expected to inherit from ActiveSupport::TestCase

All our tests still inherit from ActiveSupport::TestCase. We are using https://github.com/metaskills/minitest-spec-rails to ease the things and benefits from all the features of minitest/spec. But you can have a basic support by adding

  • require "minitest/spec" to test/test_helper.rb
  • extend extend Minitest::Spec::DSL into ActiveSupport::TestCase

Does describe work in this case? Do we need to handle it?

My proposal doesn't support describe but is able to launch a it inside a describe.
Supporting launching a group of tests from a describe would be a nice addition. But it's maybe a bit more work, and I wanted to start simply, without getting too much in the way of the initial philosophy of supporting vanilla ActiveSupport::TestCase.
Now if that's something you would be ok with, I can work on that in a follow up PR. :)

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start with this. Since Rails encourages usage of classes to define groups, I think it's fine we leave describe out for now. We can always revisit later if needed.

Thanks for the contribution!

@vinistock vinistock merged commit f32f92c into Shopify:main Feb 20, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants