Add Minitest support to TagProf #283
Conversation
docs/profilers/tag_prof.md
Outdated
| @@ -30,15 +30,42 @@ That's how a report looks like: | |||
|
|
|||
| ## Instructions | |||
|
|
|||
| TagProf can only be used with RSpec. | |||
| TagProf can be used with both RSpec and Minitest. | |||
There was a problem hiding this comment.
I'm not sure TagProf is a very relevant name for Minitest. I was wondering if I should not create another profiler called DirectoryProf and refer to it in TagProf documentation as a Minitest equivalent. What do you think ?
There was a problem hiding this comment.
Let's use the TagProf name for now. We'll probably add support for other tags beyond "type" in the future (maybe, via integration with some third-party plugins, e.g., minitest-metadat or similar).
lib/test_prof/tag_prof/minitest.rb
Outdated
| private | ||
|
|
||
| def main_folder_path(result) | ||
| return "subdirectory_not_found: #{result.source_location.first}" if absolute_path_from(result).nil? |
There was a problem hiding this comment.
I'm not sure if this is the right behavior. Maybe we want to raise if the root test directory cannot be found ?
There was a problem hiding this comment.
Answered above: let's return :__unknown__
lioneldebauge
force-pushed
the
feature/support-minitest-for-directory-profiling
branch
from
6c3e7e1
to
1a30c6e
Compare
lioneldebauge
force-pushed
the
feature/support-minitest-for-directory-profiling
branch
from
1a30c6e
to
11939ae
Compare
docs/profilers/tag_prof.md
Outdated
| @@ -30,15 +30,42 @@ That's how a report looks like: | |||
|
|
|||
| ## Instructions | |||
|
|
|||
| TagProf can only be used with RSpec. | |||
| TagProf can be used with both RSpec and Minitest. | |||
There was a problem hiding this comment.
Let's use the TagProf name for now. We'll probably add support for other tags beyond "type" in the future (maybe, via integration with some third-party plugins, e.g., minitest-metadat or similar).
lib/test_prof/tag_prof/minitest.rb
Outdated
| private | ||
|
|
||
| def main_folder_path(result) | ||
| return "subdirectory_not_found: #{result.source_location.first}" if absolute_path_from(result).nil? |
There was a problem hiding this comment.
Answered above: let's return :__unknown__
Co-authored-by: Vladimir Dementyev <dementiev.vm@gmail.com>
gemfiles/railsmaster.gemfile
Outdated
| @@ -1,6 +1,6 @@ | |||
| source "https://rubygems.org" | |||
|
|
|||
| gem "rails", github: "rails/rails" | |||
| gem "rails", "~> 7.0" | |||
There was a problem hiding this comment.
This allows to fix bugs related to a constant not being loaded in Rails. I fixed it by using only the last stable branch of version 7 but maybe there is a good reason why we don't do that already ?
Bugs were occuring both on my branch and on master
There was a problem hiding this comment.
Let's try adding require "active_support/inflector" somewhere here (before factory_bot):
test-prof/spec/support/ar_models.rb
Line 9 in 02d8f35
There was a problem hiding this comment.
@palkan done. CI is all green on my side 👍
|
@palkan thanks for the review. I have triggered CI in my fork with my GA free plan and everything should be green and suggestions/comments should be addressed now. Let me know what you think! |
|
Looks like a rubygems release was pushed, but no changelog/tag yet? |
|
Yeah, looks like my git push attempt failed. Thanks for pointing to this! Fixed now |
What is the purpose of this pull request?
Following this discussion this PR aims to add Minitest support for TagProf.
What changes did you make? (overview)
Created a
TagProfReporterclass which reproducesTestProf::TagProf::RSpecListenerbehavior. The main differences are:Is there anything you'd like reviewers to focus on?
You can read the comments for my different questions :)
Checklist