From a91d530ef397434a3cc0fafa955af0c2e0853606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Fri, 17 Oct 2025 10:01:40 -0600 Subject: [PATCH 1/6] Add analysis class --- .reek.yml | 12 + .rubocop_todo.yml | 6 +- lib/skunk.rb | 1 + lib/skunk/analysis.rb | 106 +++++++ lib/skunk/commands/status_reporter.rb | 28 +- lib/skunk/generators/html/overview.rb | 25 +- lib/skunk/generators/html/skunk_data.rb | 59 ---- .../html/templates/skunk_overview.html.erb | 10 +- lib/skunk/generators/json/simple.rb | 46 +-- test/lib/skunk/analysis_test.rb | 283 ++++++++++++++++++ test/lib/skunk/config_test.rb | 5 +- .../generators/html/path_truncator_test.rb | 2 - test/test_helper.rb | 1 + 13 files changed, 454 insertions(+), 130 deletions(-) create mode 100644 lib/skunk/analysis.rb delete mode 100644 lib/skunk/generators/html/skunk_data.rb create mode 100644 test/lib/skunk/analysis_test.rb diff --git a/.reek.yml b/.reek.yml index ad749c6..5d7fa4e 100644 --- a/.reek.yml +++ b/.reek.yml @@ -23,6 +23,8 @@ detectors: - initialize - Skunk::Cli::Application#execute - Skunk::Cli::Options::Argv#parse + - Skunk::Analysis#test_module? + - add_mock_methods_to_module TooManyInstanceVariables: exclude: - Skunk::Generator::Html::FileData @@ -38,6 +40,8 @@ detectors: - Skunk::Command::StatusSharer#share_url_empty? - Skunk::Configuration#supported_format? - Skunk::Configuration#supported_formats + - Skunk::Analysis#test_module? + - Skunk::ConfigTest#setup ManualDispatch: exclude: - Skunk::Config#self.method_missing @@ -45,3 +49,11 @@ detectors: BooleanParameter: exclude: - Skunk::Config#self.respond_to_missing? + DuplicateMethodCall: + exclude: + - Skunk::ConfigTest#test_add_format_ignores_duplicates + FeatureEnvy: + exclude: + - Skunk::Command::StatusReporter#table + - Skunk::Generator::HtmlReport#create_directories_and_files + - add_mock_methods_to_module diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e101f19..cc7705f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-10-17 11:20:00 UTC using RuboCop version 1.81.1. +# on 2025-10-17 16:20:40 UTC using RuboCop version 1.81.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -36,11 +36,11 @@ Lint/MissingSuper: Metrics/AbcSize: Max: 18 -# Offense count: 8 +# Offense count: 12 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. # AllowedMethods: refine Metrics/BlockLength: - Max: 82 + Max: 233 # Offense count: 2 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. diff --git a/lib/skunk.rb b/lib/skunk.rb index 1bafd37..0fe2197 100644 --- a/lib/skunk.rb +++ b/lib/skunk.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "skunk/version" +require "skunk/analysis" # Knows how to calculate the `SkunkScore` for each file analyzed by `RubyCritic` # and `SimpleCov` diff --git a/lib/skunk/analysis.rb b/lib/skunk/analysis.rb new file mode 100644 index 0000000..286c583 --- /dev/null +++ b/lib/skunk/analysis.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +module Skunk + # Centralized service for analyzing Skunk metrics from analysed modules. + # This class encapsulates all the core business logic for calculating + # Skunk scores, filtering test modules, and providing aggregated statistics. + # + # @example + # analysis = Skunk::Analysis.new(analysed_modules) + # puts "Total Skunk Score: #{analysis.skunk_score_total}" + # puts "Average: #{analysis.skunk_score_average}" + # puts "Worst module: #{analysis.worst_module.pathname}" + class Analysis + attr_reader :analysed_modules + + # @param analysed_modules [RubyCritic::AnalysedModulesCollection] Collection of analysed modules + def initialize(analysed_modules) + @analysed_modules = analysed_modules + end + + # Returns the count of non-test modules + # @return [Integer] + def analysed_modules_count + @analysed_modules_count ||= non_test_modules.count + end + + # Returns the total Skunk score across all non-test modules + # @return [Float] + def skunk_score_total + @skunk_score_total ||= non_test_modules.sum(&:skunk_score) + end + + # Returns the average Skunk score across all non-test modules + # @return [Float] + def skunk_score_average + return 0.0 if analysed_modules_count.zero? + + (skunk_score_total.to_d / analysed_modules_count).to_f.round(2) + end + + # Returns the total churn times cost across all non-test modules + # @return [Float] + def total_churn_times_cost + @total_churn_times_cost ||= non_test_modules.sum(&:churn_times_cost) + end + + # Returns the module with the highest Skunk score (worst performing) + # @return [RubyCritic::AnalysedModule, nil] + def worst_module + @worst_module ||= sorted_modules.first + end + + # Returns modules sorted by Skunk score in descending order (worst first) + # @return [Array] + def sorted_modules + @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! + end + + # Returns only non-test modules (excludes test and spec directories) + # @return [Array] + def non_test_modules + @non_test_modules ||= analysed_modules.reject do |a_module| + test_module?(a_module) + end + end + + # Returns a hash representation of the analysis results + # @return [Hash] + def to_hash + { + analysed_modules_count: analysed_modules_count, + skunk_score_total: skunk_score_total, + skunk_score_average: skunk_score_average, + total_churn_times_cost: total_churn_times_cost, + worst_pathname: worst_module&.pathname, + worst_score: worst_module&.skunk_score, + files: files_as_hash + } + end + + private + + # Returns files as an array of hashes (for JSON serialization) + # @return [Array] + def files_as_hash + @files_as_hash ||= sorted_modules.map(&:to_hash) + end + + # Determines if a module is a test module based on its path + # @param a_module [RubyCritic::AnalysedModule] The module to check + # @return [Boolean] + def test_module?(a_module) + pathname = a_module.pathname + module_path = pathname.dirname.to_s + filename = pathname.basename.to_s + + # Check if directory starts or ends with test/spec + directory_is_test = module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") + + # Check if filename ends with _test.rb or _spec.rb + filename_is_test = filename.end_with?("_test.rb", "_spec.rb") + + directory_is_test || filename_is_test + end + end +end diff --git a/lib/skunk/commands/status_reporter.rb b/lib/skunk/commands/status_reporter.rb index be43eef..edaa692 100644 --- a/lib/skunk/commands/status_reporter.rb +++ b/lib/skunk/commands/status_reporter.rb @@ -3,6 +3,7 @@ require "erb" require "rubycritic/commands/status_reporter" require "terminal-table" +require "skunk/analysis" module Skunk module Command @@ -10,6 +11,11 @@ module Command class StatusReporter < RubyCritic::Command::StatusReporter attr_accessor :analysed_modules + def initialize(options = {}) + super(options) + @analysis = nil + end + HEADINGS = %w[file skunk_score churn_times_cost churn cost coverage].freeze HEADINGS_WITHOUT_FILE = HEADINGS - %w[file] HEADINGS_WITHOUT_FILE_WIDTH = HEADINGS_WITHOUT_FILE.size * 17 # padding @@ -28,6 +34,7 @@ class StatusReporter < RubyCritic::Command::StatusReporter # Returns a status message with a table of all analysed_modules and # a skunk score average def update_status_message + @analysis = Skunk::Analysis.new(analysed_modules) opts = table_options.merge(headings: HEADINGS, rows: table) _ttable = Terminal::Table.new(opts) @@ -38,36 +45,27 @@ def update_status_message private def analysed_modules_count - @analysed_modules_count ||= non_test_modules.count - end - - def non_test_modules - @non_test_modules ||= analysed_modules.reject do |a_module| - module_path = a_module.pathname.dirname.to_s - module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") - end + @analysis.analysed_modules_count end def worst - @worst ||= sorted_modules.first + @analysis.worst_module end def sorted_modules - @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! + @analysis.sorted_modules end def total_skunk_score - @total_skunk_score ||= non_test_modules.sum(&:skunk_score) + @analysis.skunk_score_total end def total_churn_times_cost - non_test_modules.sum(&:churn_times_cost) + @analysis.total_churn_times_cost end def skunk_score_average - return 0 if analysed_modules_count.zero? - - (total_skunk_score.to_d / analysed_modules_count).to_f.round(2) + @analysis.skunk_score_average end def table_options diff --git a/lib/skunk/generators/html/overview.rb b/lib/skunk/generators/html/overview.rb index 1accdfe..c0da066 100644 --- a/lib/skunk/generators/html/overview.rb +++ b/lib/skunk/generators/html/overview.rb @@ -3,7 +3,8 @@ require "rubycritic/generators/html/base" require "skunk/generators/html/path_truncator" -require "skunk/generators/html/skunk_data" +require "skunk/generators/html/file_data" +require "skunk/analysis" module Skunk module Generator @@ -19,7 +20,9 @@ def self.erb_template(template_path) def initialize(analysed_modules) @analysed_modules = analysed_modules - @data = SkunkData.new(analysed_modules) + @analysis = Skunk::Analysis.new(analysed_modules) + @generated_at = Time.now.strftime("%Y-%m-%d %H:%M:%S") + @skunk_version = Skunk::VERSION end def file_name @@ -29,6 +32,24 @@ def file_name def render TEMPLATE.result(binding) end + + def analysed_modules_count + @analysis.analysed_modules_count + end + + def skunk_score_total + @analysis.skunk_score_total + end + + def skunk_score_average + @analysis.skunk_score_average + end + + def files + @files ||= @analysis.sorted_modules.map do |module_data| + FileData.new(module_data) + end + end end end end diff --git a/lib/skunk/generators/html/skunk_data.rb b/lib/skunk/generators/html/skunk_data.rb deleted file mode 100644 index 19eadb0..0000000 --- a/lib/skunk/generators/html/skunk_data.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require "skunk/generators/html/file_data" -require "skunk/generators/html/path_truncator" - -module Skunk - module Generator - module Html - # Data object for the HTML overview report - class SkunkData - attr_reader :generated_at, :skunk_version, - :analysed_modules_count, :skunk_score_total, :skunk_score_average, - :worst_pathname, :worst_score, :files - - def initialize(analysed_modules) - @analysed_modules = analysed_modules - @generated_at = Time.now.strftime("%Y-%m-%d %H:%M:%S") - @skunk_version = Skunk::VERSION - - @analysed_modules_count = non_test_modules.count - @skunk_score_total = non_test_modules.sum(&:skunk_score) - @skunk_score_average = calculate_average - @worst_pathname = PathTruncator.truncate(find_worst_module&.pathname) - @worst_score = find_worst_module&.skunk_score - @files = build_files - end - - private - - def non_test_modules - @non_test_modules ||= @analysed_modules.reject do |a_module| - module_path = a_module.pathname.dirname.to_s - module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") - end - end - - def calculate_average - return 0 if @analysed_modules_count.zero? - - (@skunk_score_total.to_d / @analysed_modules_count).round(2) - end - - def find_worst_module - @find_worst_module ||= sorted_modules.first - end - - def sorted_modules - @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! - end - - def build_files - @build_files ||= sorted_modules.map do |module_data| - FileData.new(module_data) - end - end - end - end - end -end diff --git a/lib/skunk/generators/html/templates/skunk_overview.html.erb b/lib/skunk/generators/html/templates/skunk_overview.html.erb index 9fea17e..cee421a 100644 --- a/lib/skunk/generators/html/templates/skunk_overview.html.erb +++ b/lib/skunk/generators/html/templates/skunk_overview.html.erb @@ -242,15 +242,15 @@
-
<%= @data.analysed_modules_count %>
+
<%= analysed_modules_count %>
Modules Analysed
-
<%= @data.skunk_score_total %>
+
<%= skunk_score_total %>
Total Skunk Score
-
<%= @data.skunk_score_average %>
+
<%= skunk_score_average %>
Average Skunk Score
@@ -269,7 +269,7 @@ - <% @data.files.each do |item| %> + <% files.each do |item| %> <%= item.file %> @@ -301,7 +301,7 @@
-

Generated with Skunk v<%= @data.skunk_version %> on <%= @data.generated_at %>

+

Generated with Skunk v<%= @skunk_version %> on <%= @generated_at %>

diff --git a/lib/skunk/generators/json/simple.rb b/lib/skunk/generators/json/simple.rb index 1dc2086..03e3ffe 100644 --- a/lib/skunk/generators/json/simple.rb +++ b/lib/skunk/generators/json/simple.rb @@ -3,6 +3,7 @@ require "pathname" require "rubycritic/configuration" +require "skunk/analysis" module Skunk module Generator @@ -11,6 +12,7 @@ module Json class Simple def initialize(analysed_modules) @analysed_modules = analysed_modules + @analysis = Skunk::Analysis.new(analysed_modules) end FILE_NAME = "skunk_report.json" @@ -20,14 +22,7 @@ def render end def data - { - analysed_modules_count: analysed_modules_count, - skunk_score_total: skunk_score_total, - skunk_score_average: calculate_average, - worst_pathname: find_worst_module&.pathname, - worst_score: find_worst_module&.skunk_score, - files: build_files - } + @analysis.to_hash end def file_directory @@ -37,41 +32,6 @@ def file_directory def file_pathname Pathname.new(file_directory).join(FILE_NAME) end - - private - - def analysed_modules_count - @analysed_modules_count ||= non_test_modules.count - end - - def calculate_average - return 0 if analysed_modules_count.zero? - - (skunk_score_total.to_d / analysed_modules_count).to_f.round(2) - end - - def skunk_score_total - @skunk_score_total ||= non_test_modules.sum(&:skunk_score) - end - - def non_test_modules - @non_test_modules ||= @analysed_modules.reject do |a_module| - module_path = a_module.pathname.dirname.to_s - module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") - end - end - - def find_worst_module - @find_worst_module ||= sorted_modules.first - end - - def sorted_modules - @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! - end - - def build_files - @build_files ||= sorted_modules.map(&:to_hash) - end end end end diff --git a/test/lib/skunk/analysis_test.rb b/test/lib/skunk/analysis_test.rb new file mode 100644 index 0000000..4a7d48d --- /dev/null +++ b/test/lib/skunk/analysis_test.rb @@ -0,0 +1,283 @@ +# frozen_string_literal: true + +require "test_helper" + +require "skunk/analysis" +require "skunk/rubycritic/analysed_module" + +describe Skunk::Analysis do + let(:analysed_modules) { [] } + let(:analysis) { Skunk::Analysis.new(analysed_modules) } + + describe "#initialize" do + it "accepts analysed_modules collection" do + _(analysis.analysed_modules).must_equal analysed_modules + end + end + + describe "#analysed_modules_count" do + context "with no modules" do + it "returns 0" do + _(analysis.analysed_modules_count).must_equal 0 + end + end + + context "with non-test modules" do + let(:analysed_modules) { [create_analysed_module("lib/file.rb"), create_analysed_module("app/model.rb")] } + + it "returns the count of non-test modules" do + _(analysis.analysed_modules_count).must_equal 2 + end + end + + context "with test modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file.rb"), + create_analysed_module("test/file_test.rb"), + create_analysed_module("spec/file_spec.rb") + ] + end + + it "excludes test modules from count" do + _(analysis.analysed_modules_count).must_equal 1 + end + end + end + + describe "#skunk_score_total" do + context "with no modules" do + it "returns 0" do + _(analysis.skunk_score_total).must_equal 0 + end + end + + context "with modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file1.rb", skunk_score: 10.5), + create_analysed_module("lib/file2.rb", skunk_score: 20.3) + ] + end + + it "returns the sum of skunk scores" do + _(analysis.skunk_score_total).must_equal 30.8 + end + end + + context "with test modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file.rb", skunk_score: 10.0), + create_analysed_module("test/file_test.rb", skunk_score: 50.0) + ] + end + + it "excludes test modules from total" do + _(analysis.skunk_score_total).must_equal 10.0 + end + end + end + + describe "#skunk_score_average" do + context "with no modules" do + it "returns 0" do + _(analysis.skunk_score_average).must_equal 0.0 + end + end + + context "with modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file1.rb", skunk_score: 10.0), + create_analysed_module("lib/file2.rb", skunk_score: 20.0) + ] + end + + it "returns the average skunk score" do + _(analysis.skunk_score_average).must_equal 15.0 + end + end + + context "with decimal average" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file1.rb", skunk_score: 10.0), + create_analysed_module("lib/file2.rb", skunk_score: 11.0) + ] + end + + it "rounds to 2 decimal places" do + _(analysis.skunk_score_average).must_equal 10.5 + end + end + end + + describe "#total_churn_times_cost" do + context "with no modules" do + it "returns 0" do + _(analysis.total_churn_times_cost).must_equal 0 + end + end + + context "with modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file1.rb", churn_times_cost: 5.0), + create_analysed_module("lib/file2.rb", churn_times_cost: 15.0) + ] + end + + it "returns the sum of churn times cost" do + _(analysis.total_churn_times_cost).must_equal 20.0 + end + end + end + + describe "#worst_module" do + context "with no modules" do + it "returns nil" do + _(analysis.worst_module).must_be_nil + end + end + + context "with modules" do + let(:worst_module) { create_analysed_module("lib/worst.rb", skunk_score: 100.0) } + let(:best_module) { create_analysed_module("lib/best.rb", skunk_score: 10.0) } + let(:analysed_modules) { [best_module, worst_module] } + + it "returns the module with highest skunk score" do + _(analysis.worst_module).must_equal worst_module + end + end + end + + describe "#sorted_modules" do + context "with no modules" do + it "returns empty array" do + _(analysis.sorted_modules).must_equal [] + end + end + + context "with modules" do + let(:module1) { create_analysed_module("lib/file1.rb", skunk_score: 10.0) } + let(:module2) { create_analysed_module("lib/file2.rb", skunk_score: 30.0) } + let(:module3) { create_analysed_module("lib/file3.rb", skunk_score: 20.0) } + let(:analysed_modules) { [module1, module2, module3] } + + it "returns modules sorted by skunk score descending" do + _(analysis.sorted_modules).must_equal [module2, module3, module1] + end + end + + context "with test modules" do + let(:spec_module) { create_analysed_module("test/file_test.rb", skunk_score: 100.0) } + let(:lib_module) { create_analysed_module("lib/file.rb", skunk_score: 10.0) } + let(:analysed_modules) { [spec_module, lib_module] } + + it "excludes test modules from sorted list" do + _(analysis.sorted_modules).must_equal [lib_module] + end + end + end + + describe "#non_test_modules" do + context "with mixed modules" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file.rb"), + create_analysed_module("test/file_test.rb"), + create_analysed_module("spec/file_spec.rb"), + create_analysed_module("app/model.rb") + ] + end + + it "filters out test and spec modules" do + non_test = analysis.non_test_modules + _(non_test.size).must_equal 2 + _(non_test.map(&:pathname).map(&:to_s)).must_include "lib/file.rb" + _(non_test.map(&:pathname).map(&:to_s)).must_include "app/model.rb" + end + end + + context "with modules in test directories" do + let(:analysed_modules) do + [ + create_analysed_module("test/unit/file.rb"), + create_analysed_module("spec/unit/file.rb"), + create_analysed_module("lib/file.rb") + ] + end + + it "filters out modules in test directories" do + non_test = analysis.non_test_modules + _(non_test.size).must_equal 1 + _(non_test.first.pathname.to_s).must_equal "lib/file.rb" + end + end + + context "with modules ending in test/spec" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file_test.rb"), + create_analysed_module("lib/file_spec.rb"), + create_analysed_module("lib/file.rb") + ] + end + + it "filters out modules ending in test/spec" do + non_test = analysis.non_test_modules + _(non_test.size).must_equal 1 + _(non_test.first.pathname.to_s).must_equal "lib/file.rb" + end + end + end + + describe "#to_hash" do + let(:analysed_modules) do + [ + create_analysed_module("lib/file.rb", skunk_score: 10.0, churn_times_cost: 5.0) + ] + end + + it "returns a hash with all analysis data including files" do + hash = analysis.to_hash + _(hash[:analysed_modules_count]).must_equal 1 + _(hash[:skunk_score_total]).must_equal 10.0 + _(hash[:skunk_score_average]).must_equal 10.0 + _(hash[:total_churn_times_cost]).must_equal 5.0 + _(hash[:worst_pathname]).must_equal Pathname.new("lib/file.rb") + _(hash[:worst_score]).must_equal 10.0 + _(hash[:files]).must_be_kind_of Array + _(hash[:files].size).must_equal 1 + _(hash[:files].first[:file]).must_equal "lib/file.rb" + _(hash[:files].first[:skunk_score]).must_equal 10.0 + end + end + + private + + def create_analysed_module(path, skunk_score: 0.0, churn_times_cost: 0.0) + module_path = Pathname.new(path) + analysed_module = RubyCritic::AnalysedModule.new( + pathname: module_path, + smells: [], + churn: 1, + committed_at: Time.now + ) + + add_mock_methods_to_module(analysed_module, skunk_score, churn_times_cost) + end + + def add_mock_methods_to_module(analysed_module, skunk_score, churn_times_cost) + # Mock the skunk_score and churn_times_cost methods + analysed_module.define_singleton_method(:skunk_score) { @skunk_score ||= 0.0 } + analysed_module.define_singleton_method(:skunk_score=) { |value| @skunk_score = value } + analysed_module.define_singleton_method(:churn_times_cost) { @churn_times_cost ||= 0.0 } + analysed_module.define_singleton_method(:churn_times_cost=) { |value| @churn_times_cost = value } + + analysed_module.skunk_score = skunk_score + analysed_module.churn_times_cost = churn_times_cost + analysed_module + end +end diff --git a/test/lib/skunk/config_test.rb b/test/lib/skunk/config_test.rb index ceaf304..6395f61 100644 --- a/test/lib/skunk/config_test.rb +++ b/test/lib/skunk/config_test.rb @@ -4,7 +4,10 @@ require_relative "../../../lib/skunk/config" module Skunk + # Test class for Skunk::Config functionality class ConfigTest < Minitest::Test + # Reset configuration before each test to ensure clean state + # This method doesn't depend on instance state as it's a class-level operation def setup Config.reset end @@ -40,7 +43,7 @@ def test_add_format def test_add_format_ignores_duplicates Config.add_format(:html) - Config.add_format(:html) + Config.add_format(:html) # This should be ignored as duplicate assert_equal %i[json html], Config.formats end diff --git a/test/lib/skunk/generators/html/path_truncator_test.rb b/test/lib/skunk/generators/html/path_truncator_test.rb index e8236ee..695ab4e 100644 --- a/test/lib/skunk/generators/html/path_truncator_test.rb +++ b/test/lib/skunk/generators/html/path_truncator_test.rb @@ -4,7 +4,6 @@ require "skunk/generators/html/path_truncator" -# rubocop:disable Metrics/BlockLength describe Skunk::Generator::Html::PathTruncator do describe ".truncate" do context "when path contains app folder" do @@ -176,4 +175,3 @@ end end end -# rubocop:enable Metrics/BlockLength diff --git a/test/test_helper.rb b/test/test_helper.rb index 1b63517..3d4c2f1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -27,6 +27,7 @@ require "minitest/stub_any_instance" require "webmock/minitest" +require "skunk" require "skunk/rubycritic/analysed_module" def context(*args, &block) From 9a233feb9f6a6e48e13cc73d0a39bd090134414f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Fri, 17 Oct 2025 10:56:53 -0600 Subject: [PATCH 2/6] Move from class to module Refactor Skunk analysis integration by removing the Analysis class and adding its methods directly to RubyCritic::AnalysedModulesCollection. Update related files to utilize the new methods for Skunk score calculations and reporting. --- lib/skunk.rb | 1 - lib/skunk/analysis.rb | 106 ------------------ lib/skunk/commands/status_reporter.rb | 15 +-- lib/skunk/generators/html/overview.rb | 11 +- lib/skunk/generators/json/simple.rb | 5 +- .../rubycritic/analysed_modules_collection.rb | 97 +++++++++++++++- .../skunk/commands/status_reporter_test.rb | 1 + .../analysed_modules_collection_test.rb} | 60 +++++----- 8 files changed, 136 insertions(+), 160 deletions(-) delete mode 100644 lib/skunk/analysis.rb rename test/lib/skunk/{analysis_test.rb => rubycritic/analysed_modules_collection_test.rb} (82%) diff --git a/lib/skunk.rb b/lib/skunk.rb index 0fe2197..1bafd37 100644 --- a/lib/skunk.rb +++ b/lib/skunk.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "skunk/version" -require "skunk/analysis" # Knows how to calculate the `SkunkScore` for each file analyzed by `RubyCritic` # and `SimpleCov` diff --git a/lib/skunk/analysis.rb b/lib/skunk/analysis.rb deleted file mode 100644 index 286c583..0000000 --- a/lib/skunk/analysis.rb +++ /dev/null @@ -1,106 +0,0 @@ -# frozen_string_literal: true - -module Skunk - # Centralized service for analyzing Skunk metrics from analysed modules. - # This class encapsulates all the core business logic for calculating - # Skunk scores, filtering test modules, and providing aggregated statistics. - # - # @example - # analysis = Skunk::Analysis.new(analysed_modules) - # puts "Total Skunk Score: #{analysis.skunk_score_total}" - # puts "Average: #{analysis.skunk_score_average}" - # puts "Worst module: #{analysis.worst_module.pathname}" - class Analysis - attr_reader :analysed_modules - - # @param analysed_modules [RubyCritic::AnalysedModulesCollection] Collection of analysed modules - def initialize(analysed_modules) - @analysed_modules = analysed_modules - end - - # Returns the count of non-test modules - # @return [Integer] - def analysed_modules_count - @analysed_modules_count ||= non_test_modules.count - end - - # Returns the total Skunk score across all non-test modules - # @return [Float] - def skunk_score_total - @skunk_score_total ||= non_test_modules.sum(&:skunk_score) - end - - # Returns the average Skunk score across all non-test modules - # @return [Float] - def skunk_score_average - return 0.0 if analysed_modules_count.zero? - - (skunk_score_total.to_d / analysed_modules_count).to_f.round(2) - end - - # Returns the total churn times cost across all non-test modules - # @return [Float] - def total_churn_times_cost - @total_churn_times_cost ||= non_test_modules.sum(&:churn_times_cost) - end - - # Returns the module with the highest Skunk score (worst performing) - # @return [RubyCritic::AnalysedModule, nil] - def worst_module - @worst_module ||= sorted_modules.first - end - - # Returns modules sorted by Skunk score in descending order (worst first) - # @return [Array] - def sorted_modules - @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! - end - - # Returns only non-test modules (excludes test and spec directories) - # @return [Array] - def non_test_modules - @non_test_modules ||= analysed_modules.reject do |a_module| - test_module?(a_module) - end - end - - # Returns a hash representation of the analysis results - # @return [Hash] - def to_hash - { - analysed_modules_count: analysed_modules_count, - skunk_score_total: skunk_score_total, - skunk_score_average: skunk_score_average, - total_churn_times_cost: total_churn_times_cost, - worst_pathname: worst_module&.pathname, - worst_score: worst_module&.skunk_score, - files: files_as_hash - } - end - - private - - # Returns files as an array of hashes (for JSON serialization) - # @return [Array] - def files_as_hash - @files_as_hash ||= sorted_modules.map(&:to_hash) - end - - # Determines if a module is a test module based on its path - # @param a_module [RubyCritic::AnalysedModule] The module to check - # @return [Boolean] - def test_module?(a_module) - pathname = a_module.pathname - module_path = pathname.dirname.to_s - filename = pathname.basename.to_s - - # Check if directory starts or ends with test/spec - directory_is_test = module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") - - # Check if filename ends with _test.rb or _spec.rb - filename_is_test = filename.end_with?("_test.rb", "_spec.rb") - - directory_is_test || filename_is_test - end - end -end diff --git a/lib/skunk/commands/status_reporter.rb b/lib/skunk/commands/status_reporter.rb index edaa692..315347a 100644 --- a/lib/skunk/commands/status_reporter.rb +++ b/lib/skunk/commands/status_reporter.rb @@ -3,7 +3,6 @@ require "erb" require "rubycritic/commands/status_reporter" require "terminal-table" -require "skunk/analysis" module Skunk module Command @@ -13,7 +12,6 @@ class StatusReporter < RubyCritic::Command::StatusReporter def initialize(options = {}) super(options) - @analysis = nil end HEADINGS = %w[file skunk_score churn_times_cost churn cost coverage].freeze @@ -34,7 +32,6 @@ def initialize(options = {}) # Returns a status message with a table of all analysed_modules and # a skunk score average def update_status_message - @analysis = Skunk::Analysis.new(analysed_modules) opts = table_options.merge(headings: HEADINGS, rows: table) _ttable = Terminal::Table.new(opts) @@ -45,27 +42,27 @@ def update_status_message private def analysed_modules_count - @analysis.analysed_modules_count + analysed_modules.analysed_modules_count end def worst - @analysis.worst_module + analysed_modules.worst_module end def sorted_modules - @analysis.sorted_modules + analysed_modules.sorted_modules end def total_skunk_score - @analysis.skunk_score_total + analysed_modules.skunk_score_total end def total_churn_times_cost - @analysis.total_churn_times_cost + analysed_modules.total_churn_times_cost end def skunk_score_average - @analysis.skunk_score_average + analysed_modules.skunk_score_average end def table_options diff --git a/lib/skunk/generators/html/overview.rb b/lib/skunk/generators/html/overview.rb index c0da066..78e7d2b 100644 --- a/lib/skunk/generators/html/overview.rb +++ b/lib/skunk/generators/html/overview.rb @@ -4,7 +4,7 @@ require "skunk/generators/html/path_truncator" require "skunk/generators/html/file_data" -require "skunk/analysis" +require "skunk/rubycritic/analysed_modules_collection" module Skunk module Generator @@ -20,7 +20,6 @@ def self.erb_template(template_path) def initialize(analysed_modules) @analysed_modules = analysed_modules - @analysis = Skunk::Analysis.new(analysed_modules) @generated_at = Time.now.strftime("%Y-%m-%d %H:%M:%S") @skunk_version = Skunk::VERSION end @@ -34,19 +33,19 @@ def render end def analysed_modules_count - @analysis.analysed_modules_count + @analysed_modules.analysed_modules_count end def skunk_score_total - @analysis.skunk_score_total + @analysed_modules.skunk_score_total end def skunk_score_average - @analysis.skunk_score_average + @analysed_modules.skunk_score_average end def files - @files ||= @analysis.sorted_modules.map do |module_data| + @files ||= @analysed_modules.sorted_modules.map do |module_data| FileData.new(module_data) end end diff --git a/lib/skunk/generators/json/simple.rb b/lib/skunk/generators/json/simple.rb index 03e3ffe..0a686ce 100644 --- a/lib/skunk/generators/json/simple.rb +++ b/lib/skunk/generators/json/simple.rb @@ -3,7 +3,7 @@ require "pathname" require "rubycritic/configuration" -require "skunk/analysis" +require "skunk/rubycritic/analysed_modules_collection" module Skunk module Generator @@ -12,7 +12,6 @@ module Json class Simple def initialize(analysed_modules) @analysed_modules = analysed_modules - @analysis = Skunk::Analysis.new(analysed_modules) end FILE_NAME = "skunk_report.json" @@ -22,7 +21,7 @@ def render end def data - @analysis.to_hash + @analysed_modules.to_hash end def file_directory diff --git a/lib/skunk/rubycritic/analysed_modules_collection.rb b/lib/skunk/rubycritic/analysed_modules_collection.rb index 297aaf0..fc3f33b 100644 --- a/lib/skunk/rubycritic/analysed_modules_collection.rb +++ b/lib/skunk/rubycritic/analysed_modules_collection.rb @@ -3,15 +3,100 @@ require "rubycritic/core/analysed_modules_collection" module RubyCritic - # nodoc # + # Monkey-patches RubyCritic::AnalysedModulesCollection to add Skunk analysis methods class AnalysedModulesCollection + # Returns the count of non-test modules + # @return [Integer] + def analysed_modules_count + @analysed_modules_count ||= non_test_modules.count + end + + # Returns the total Skunk score across all non-test modules + # @return [Float] + def skunk_score_total + @skunk_score_total ||= non_test_modules.sum(&:skunk_score) + end + + # Returns the average Skunk score across all non-test modules + # @return [Float] def skunk_score_average - num_modules = @modules.size - if num_modules.positive? - sum(&:skunk_score) / num_modules.to_f - else - 0.0 + return 0.0 if analysed_modules_count.zero? + + (skunk_score_total.to_d / analysed_modules_count).to_f.round(2) + end + + # Returns the total churn times cost across all non-test modules + # @return [Float] + def total_churn_times_cost + @total_churn_times_cost ||= non_test_modules.sum(&:churn_times_cost) + end + + # Returns the module with the highest Skunk score (worst performing) + # @return [RubyCritic::AnalysedModule, nil] + def worst_module + @worst_module ||= sorted_modules.first + end + + # Returns modules sorted by Skunk score in descending order (worst first) + # @return [Array] + def sorted_modules + @sorted_modules ||= non_test_modules.sort_by(&:skunk_score).reverse! + end + + # Returns only non-test modules (excludes test and spec directories) + # @return [Array] + def non_test_modules + @non_test_modules ||= reject do |a_module| + test_module?(a_module) end end + + # Returns a hash representation of the analysis results + # @return [Hash] + def to_hash + { + analysed_modules_count: analysed_modules_count, + skunk_score_total: skunk_score_total, + skunk_score_average: skunk_score_average, + total_churn_times_cost: total_churn_times_cost, + worst_pathname: worst_module&.pathname, + worst_score: worst_module&.skunk_score, + files: files_as_hash + } + end + + private + + # Returns files as an array of hashes (for JSON serialization) + # @return [Array] + def files_as_hash + @files_as_hash ||= sorted_modules.map(&:to_hash) + end + + # Determines if a module is a test module based on its path + # @param a_module [RubyCritic::AnalysedModule] The module to check + # @return [Boolean] + def test_module?(a_module) + pathname = a_module.pathname + directory_is_test?(pathname) || filename_is_test?(pathname) + end + + # Checks if the directory path indicates a test module + # @param pathname [Pathname] The pathname to check + # @return [Boolean] + # :reek:UtilityFunction + def directory_is_test?(pathname) + module_path = pathname.dirname.to_s + module_path.start_with?("test", "spec") || module_path.end_with?("test", "spec") + end + + # Checks if the filename indicates a test module + # @param pathname [Pathname] The pathname to check + # @return [Boolean] + # :reek:UtilityFunction + def filename_is_test?(pathname) + filename = pathname.basename.to_s + filename.end_with?("_test.rb", "_spec.rb") + end end end diff --git a/test/lib/skunk/commands/status_reporter_test.rb b/test/lib/skunk/commands/status_reporter_test.rb index 92302d9..26585ab 100644 --- a/test/lib/skunk/commands/status_reporter_test.rb +++ b/test/lib/skunk/commands/status_reporter_test.rb @@ -3,6 +3,7 @@ require "test_helper" require "rubycritic/analysers_runner" +require "skunk/rubycritic/analysed_modules_collection" require "skunk/commands/status_reporter" describe Skunk::Command::StatusReporter do diff --git a/test/lib/skunk/analysis_test.rb b/test/lib/skunk/rubycritic/analysed_modules_collection_test.rb similarity index 82% rename from test/lib/skunk/analysis_test.rb rename to test/lib/skunk/rubycritic/analysed_modules_collection_test.rb index 4a7d48d..5bf4e92 100644 --- a/test/lib/skunk/analysis_test.rb +++ b/test/lib/skunk/rubycritic/analysed_modules_collection_test.rb @@ -2,23 +2,17 @@ require "test_helper" -require "skunk/analysis" +require "skunk/rubycritic/analysed_modules_collection" require "skunk/rubycritic/analysed_module" -describe Skunk::Analysis do +describe RubyCritic::AnalysedModulesCollection do let(:analysed_modules) { [] } - let(:analysis) { Skunk::Analysis.new(analysed_modules) } - - describe "#initialize" do - it "accepts analysed_modules collection" do - _(analysis.analysed_modules).must_equal analysed_modules - end - end + let(:collection) { create_collection(analysed_modules) } describe "#analysed_modules_count" do context "with no modules" do it "returns 0" do - _(analysis.analysed_modules_count).must_equal 0 + _(collection.analysed_modules_count).must_equal 0 end end @@ -26,7 +20,7 @@ let(:analysed_modules) { [create_analysed_module("lib/file.rb"), create_analysed_module("app/model.rb")] } it "returns the count of non-test modules" do - _(analysis.analysed_modules_count).must_equal 2 + _(collection.analysed_modules_count).must_equal 2 end end @@ -40,7 +34,7 @@ end it "excludes test modules from count" do - _(analysis.analysed_modules_count).must_equal 1 + _(collection.analysed_modules_count).must_equal 1 end end end @@ -48,7 +42,7 @@ describe "#skunk_score_total" do context "with no modules" do it "returns 0" do - _(analysis.skunk_score_total).must_equal 0 + _(collection.skunk_score_total).must_equal 0 end end @@ -61,7 +55,7 @@ end it "returns the sum of skunk scores" do - _(analysis.skunk_score_total).must_equal 30.8 + _(collection.skunk_score_total).must_equal 30.8 end end @@ -74,7 +68,7 @@ end it "excludes test modules from total" do - _(analysis.skunk_score_total).must_equal 10.0 + _(collection.skunk_score_total).must_equal 10.0 end end end @@ -82,7 +76,7 @@ describe "#skunk_score_average" do context "with no modules" do it "returns 0" do - _(analysis.skunk_score_average).must_equal 0.0 + _(collection.skunk_score_average).must_equal 0.0 end end @@ -95,7 +89,7 @@ end it "returns the average skunk score" do - _(analysis.skunk_score_average).must_equal 15.0 + _(collection.skunk_score_average).must_equal 15.0 end end @@ -108,7 +102,7 @@ end it "rounds to 2 decimal places" do - _(analysis.skunk_score_average).must_equal 10.5 + _(collection.skunk_score_average).must_equal 10.5 end end end @@ -116,7 +110,7 @@ describe "#total_churn_times_cost" do context "with no modules" do it "returns 0" do - _(analysis.total_churn_times_cost).must_equal 0 + _(collection.total_churn_times_cost).must_equal 0 end end @@ -129,7 +123,7 @@ end it "returns the sum of churn times cost" do - _(analysis.total_churn_times_cost).must_equal 20.0 + _(collection.total_churn_times_cost).must_equal 20.0 end end end @@ -137,7 +131,7 @@ describe "#worst_module" do context "with no modules" do it "returns nil" do - _(analysis.worst_module).must_be_nil + _(collection.worst_module).must_be_nil end end @@ -147,7 +141,7 @@ let(:analysed_modules) { [best_module, worst_module] } it "returns the module with highest skunk score" do - _(analysis.worst_module).must_equal worst_module + _(collection.worst_module).must_equal worst_module end end end @@ -155,7 +149,7 @@ describe "#sorted_modules" do context "with no modules" do it "returns empty array" do - _(analysis.sorted_modules).must_equal [] + _(collection.sorted_modules).must_equal [] end end @@ -166,7 +160,7 @@ let(:analysed_modules) { [module1, module2, module3] } it "returns modules sorted by skunk score descending" do - _(analysis.sorted_modules).must_equal [module2, module3, module1] + _(collection.sorted_modules).must_equal [module2, module3, module1] end end @@ -176,7 +170,7 @@ let(:analysed_modules) { [spec_module, lib_module] } it "excludes test modules from sorted list" do - _(analysis.sorted_modules).must_equal [lib_module] + _(collection.sorted_modules).must_equal [lib_module] end end end @@ -193,7 +187,7 @@ end it "filters out test and spec modules" do - non_test = analysis.non_test_modules + non_test = collection.non_test_modules _(non_test.size).must_equal 2 _(non_test.map(&:pathname).map(&:to_s)).must_include "lib/file.rb" _(non_test.map(&:pathname).map(&:to_s)).must_include "app/model.rb" @@ -210,7 +204,7 @@ end it "filters out modules in test directories" do - non_test = analysis.non_test_modules + non_test = collection.non_test_modules _(non_test.size).must_equal 1 _(non_test.first.pathname.to_s).must_equal "lib/file.rb" end @@ -226,7 +220,7 @@ end it "filters out modules ending in test/spec" do - non_test = analysis.non_test_modules + non_test = collection.non_test_modules _(non_test.size).must_equal 1 _(non_test.first.pathname.to_s).must_equal "lib/file.rb" end @@ -241,7 +235,7 @@ end it "returns a hash with all analysis data including files" do - hash = analysis.to_hash + hash = collection.to_hash _(hash[:analysed_modules_count]).must_equal 1 _(hash[:skunk_score_total]).must_equal 10.0 _(hash[:skunk_score_average]).must_equal 10.0 @@ -257,6 +251,14 @@ private + def create_collection(modules) + # Create a collection by manually setting the @modules instance variable + # This bypasses the complex initialization that expects file paths + collection = RubyCritic::AnalysedModulesCollection.new([], []) + collection.instance_variable_set(:@modules, modules) + collection + end + def create_analysed_module(path, skunk_score: 0.0, churn_times_cost: 0.0) module_path = Pathname.new(path) analysed_module = RubyCritic::AnalysedModule.new( From 126345ae370d13029d7040adeca988e9f81a67f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Fri, 17 Oct 2025 11:30:47 -0600 Subject: [PATCH 3/6] Add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cbe885..2dc4b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## main [(unreleased)](https://github.com/fastruby/skunk/compare/v0.5.4...HEAD) +* [REFACTOR: Move Skunk analysis to RubyCritic module](https://github.com/fastruby/skunk/pull/127) * [FEATURE: Add Skunk HTML Report](https://github.com/fastruby/skunk/pull/123) * [FEATURE: Add Skunk::Config class](https://github.com/fastruby/skunk/pull/123) * [FEATURE: Add Ruby 3.4 compatibility](https://github.com/fastruby/skunk/pull/124) From ad1e6d2017fd6fa5d193162e511446c889114d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Fri, 17 Oct 2025 11:34:11 -0600 Subject: [PATCH 4/6] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- test/lib/skunk/config_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/skunk/config_test.rb b/test/lib/skunk/config_test.rb index 6395f61..3664d63 100644 --- a/test/lib/skunk/config_test.rb +++ b/test/lib/skunk/config_test.rb @@ -7,7 +7,6 @@ module Skunk # Test class for Skunk::Config functionality class ConfigTest < Minitest::Test # Reset configuration before each test to ensure clean state - # This method doesn't depend on instance state as it's a class-level operation def setup Config.reset end From 59fb32d5aad70b6c330a942225130f84a6064706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Fri, 17 Oct 2025 11:34:44 -0600 Subject: [PATCH 5/6] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/skunk/rubycritic/analysed_modules_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/skunk/rubycritic/analysed_modules_collection.rb b/lib/skunk/rubycritic/analysed_modules_collection.rb index fc3f33b..d03fc05 100644 --- a/lib/skunk/rubycritic/analysed_modules_collection.rb +++ b/lib/skunk/rubycritic/analysed_modules_collection.rb @@ -3,7 +3,7 @@ require "rubycritic/core/analysed_modules_collection" module RubyCritic - # Monkey-patches RubyCritic::AnalysedModulesCollection to add Skunk analysis methods + # Extends RubyCritic::AnalysedModulesCollection to add Skunk analysis methods class AnalysedModulesCollection # Returns the count of non-test modules # @return [Integer] From c72a9c3ce843f47741833bca4f104c5d23ab1341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20V=C3=A1squez?= Date: Thu, 23 Oct 2025 15:27:44 -0600 Subject: [PATCH 6/6] Update CHANGELOG.md Co-authored-by: Francois --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dc4b80..e6bc057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## main [(unreleased)](https://github.com/fastruby/skunk/compare/v0.5.4...HEAD) -* [REFACTOR: Move Skunk analysis to RubyCritic module](https://github.com/fastruby/skunk/pull/127) +* [REFACTOR: Centralize Skunk analysis into RubyCritic module](https://github.com/fastruby/skunk/pull/127) * [FEATURE: Add Skunk HTML Report](https://github.com/fastruby/skunk/pull/123) * [FEATURE: Add Skunk::Config class](https://github.com/fastruby/skunk/pull/123) * [FEATURE: Add Ruby 3.4 compatibility](https://github.com/fastruby/skunk/pull/124)