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

Register tests directory for rails stats #2129

Merged
merged 5 commits into from
Oct 15, 2024

Conversation

javierav
Copy link
Contributor

@javierav javierav commented Oct 9, 2024

What are you trying to accomplish?

After the introduction of PR #2081, the tests directory was missing in rails stats.

Copy link
Member

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

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

@javierav thanks for writing up a PR here, but I'm confused: are you saying that previously rails stats would include tests for components? I don't remember that ever being the case.

@javierav
Copy link
Contributor Author

javierav commented Oct 9, 2024

@javierav thanks for writing up a PR here, but I'm confused: are you saying that previously rails stats would include tests for components? I don't remember that ever being the case.

No, sorry for the confusion. What I meant was that the previous PR adds the "app/components" folder to rails stats but it forgot to add the "test/components" folder if it exists, which is what this PR does.

@joelhawksley
Copy link
Member

@javierav got it. I'm hesitant to add test/components if it wasn't there before. I believe rails stats is meant to report on the contents of app. Have you seen it report on other test-related objects?

@javierav
Copy link
Contributor Author

javierav commented Oct 9, 2024

@joelhawksley sure! For example already exists in Rails 6.0 or RSpec

This is the output for a Rails 7.2 empty app:

+----------------------+--------+--------+---------+---------+-----+-------+
| Name                 |  Lines |    LOC | Classes | Methods | M/C | LOC/M |
+----------------------+--------+--------+---------+---------+-----+-------+
| Controllers          |      4 |      3 |       1 |       0 |   0 |     0 |
| Helpers              |      2 |      2 |       0 |       0 |   0 |     0 |
| Jobs                 |      7 |      2 |       1 |       0 |   0 |     0 |
| Models               |      3 |      3 |       1 |       0 |   0 |     0 |
| Mailers              |      4 |      4 |       1 |       0 |   0 |     0 |
| Channels             |      8 |      8 |       2 |       0 |   0 |     0 |
| Views                |     85 |     55 |       0 |       1 |   0 |    53 |
| Stylesheets          |     15 |     15 |       0 |       0 |   0 |     0 |
| JavaScript           |     23 |     16 |       0 |       0 |   0 |     0 |
| Libraries            |      0 |      0 |       0 |       0 |   0 |     0 |
| Controller tests     |      0 |      0 |       0 |       0 |   0 |     0 |
| Helper tests         |      0 |      0 |       0 |       0 |   0 |     0 |
| Model tests          |      0 |      0 |       0 |       0 |   0 |     0 |
| Mailer tests         |      0 |      0 |       0 |       0 |   0 |     0 |
| Channel tests        |     13 |      5 |       1 |       0 |   0 |     0 |
| Integration tests    |      0 |      0 |       0 |       0 |   0 |     0 |
| System tests         |      0 |      0 |       0 |       0 |   0 |     0 |
+----------------------+--------+--------+---------+---------+-----+-------+
| Total                |    164 |    113 |       7 |       1 |   0 |   111 |
+----------------------+--------+--------+---------+---------+-----+-------+
  Code LOC: 108     Test LOC: 5     Code to Test Ratio: 1:0.0

@joelhawksley
Copy link
Member

@javierav no kidding! Well, then let's figure out how to land this one ❤️

Have you tested it with rspec? It looks like we're hardcoding test/components, but wouldn't it be spec/components if rspec is being used?

@javierav
Copy link
Contributor Author

javierav commented Oct 9, 2024

@joelhawksley Yes, initially the test/components path is hardcoded because there is no such thing as ViewComponent::Base.view_component_path for tests.

In this case, we just need to check if the project is using Rails default tests (minitest) to add the test/components path, since if the project is using RSpec, RSpec itself will be in charge of adding its paths to the stats here

Unfortunately, Rails only adds a series of initial directories to the stats, and test/components is not among them.

docs/CHANGELOG.md Outdated Show resolved Hide resolved
docs/CHANGELOG.md Outdated Show resolved Hide resolved
@joelhawksley joelhawksley enabled auto-merge (squash) October 15, 2024 16:03
@joelhawksley joelhawksley merged commit 212d159 into ViewComponent:main Oct 15, 2024
19 checks passed
@javierav javierav deleted the rails-stats-tests branch October 16, 2024 09:47
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.

2 participants