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

[CIVIS-9349] fix minitest instrumentation with mixins #134

Merged
merged 8 commits into from
Mar 12, 2024

Conversation

anmarchenko
Copy link
Member

@anmarchenko anmarchenko commented Mar 11, 2024

What does this PR do?
Fixes #130.

Uses Ruby's Module.const_source_location method that is available since Ruby 2.7 to determine the source location of Minitest::Runnable subclass (test suite).

Motivation
Bug reported in the linked issue shows that we did not pay enough attention to the fact that Minitest allows tests to be defined in shared modules and be mixed in any test suite. The current way to determine test suite's source location is brittle because it returns method's location which could be anything in this case. This worked fine for tests that didn't use mixins and for tests that used only mixins, but broke for tests that used both mixed in tests and normal tests.

Proposed fix is to use where the class constant is defined as test suite location (and method only as fallback). As we target Ruby 2.7+ support since GA, it is safe to use and works for all cases.

Tests
Tests using ActiveSupport and mixins are provided

@anmarchenko anmarchenko marked this pull request as ready for review March 11, 2024 16:06
@anmarchenko anmarchenko requested review from a team as code owners March 11, 2024 16:06
@anmarchenko anmarchenko requested a review from tonyredondo March 11, 2024 16:06
@anmarchenko anmarchenko changed the title fix minitest instrumentation with mixins [CIVIS-9349] fix minitest instrumentation with mixins Mar 11, 2024
@anmarchenko anmarchenko force-pushed the anmarchenko/minitest_ignored_tests branch from c637993 to e295925 Compare March 12, 2024 08:20
@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 99.25926% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 99.08%. Comparing base (f71d45c) to head (570e56c).

Files Patch % Lines
...g/ci/contrib/minitest_shoulda_context/fake_test.rb 96.15% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main     #134    +/-   ##
========================================
  Coverage   99.08%   99.08%            
========================================
  Files         156      164     +8     
  Lines        7102     7236   +134     
  Branches      300      304     +4     
========================================
+ Hits         7037     7170   +133     
- Misses         65       66     +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anmarchenko anmarchenko force-pushed the anmarchenko/minitest_ignored_tests branch from eb75332 to 570e56c Compare March 12, 2024 08:28
@anmarchenko anmarchenko added this to the 0.8.1 milestone Mar 12, 2024
@anmarchenko anmarchenko merged commit 9fc3bfe into main Mar 12, 2024
26 checks passed
@anmarchenko anmarchenko deleted the anmarchenko/minitest_ignored_tests branch March 12, 2024 09:20
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.

Invalid events being skipped when using with Minitest due to a missing test suite ID
3 participants