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

Mark methods called by MiniTest.setup as alive #468

Merged
merged 1 commit into from Mar 12, 2024
Merged

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Sep 20, 2023

@Morriar Morriar added the bugfix Fix a bug label Sep 20, 2023
@Morriar Morriar requested a review from a team as a code owner September 20, 2023 20:20
@Morriar Morriar self-assigned this Sep 20, 2023
Copy link
Member

@kmcphillips kmcphillips left a comment

Choose a reason for hiding this comment

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

Nice. Yes I think this is good. We use ActiveSupport::TestCase which is a subclass of Minitest::Test.

@andyw8
Copy link
Contributor

andyw8 commented Sep 20, 2023

Interesting, is that part of the public API? I don't see it in the docs.


sig { override.params(indexer: Indexer, send: Send).void }
def on_send(indexer, send)
return unless send.recv.nil? && send.name == "setup"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also consider teardown?

@Morriar
Copy link
Collaborator Author

Morriar commented Oct 3, 2023

Indeed, it's a ActiveSupport::TestCase and not a Minitest one 🤦

I moved the rule to the right file and also handled teardown as @andyw8 pointed out.

@greatscotty greatscotty deleted the at-fix-dead-setup branch February 13, 2024 00:07
@greatscotty greatscotty restored the at-fix-dead-setup branch February 13, 2024 00:07
@greatscotty greatscotty deleted the at-fix-dead-setup branch February 13, 2024 00:07
@greatscotty greatscotty restored the at-fix-dead-setup branch February 13, 2024 00:07
@greatscotty
Copy link

Accidentally hit this branch. Please make sure to rebase this branch to the newest version before merging

@Morriar Morriar reopened this Mar 12, 2024
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar enabled auto-merge March 12, 2024 14:15
@Morriar Morriar merged commit 7569062 into main Mar 12, 2024
9 checks passed
@Morriar Morriar deleted the at-fix-dead-setup branch March 12, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants