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

Add stats executable for reporting repo state. #1541

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions lib/sass_spec/stats.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
# encoding: utf-8

require 'command_line_reporter'
require 'pathname'
require 'yaml'

class SassSpec::Stats::CLI
include CommandLineReporter

def self.parse(args)
options = {}
parser = OptionParser.new
parser.banner = <<BANNER
Usage: ./stats.rb [options] [directories...]

This command prints stats about the repo.

BANNER

parser.on("--todo IMPLEMENTATION",
"Find specs that have a todo for the given implementation.") do |impl|
options[:todo] = impl
end

parser.on("--issue GITHUB_ISSUE",
"Find specs that correspond to a Github issue.") do |issue|
unless issue =~ /.+#\d+/
warn("Invalid issue.\n\n")
puts parser.help()
end
issue = issue.split("#")
number = issue.pop
issue = issue.join().split("/")
impl = issue.pop
options[:issue] = "#{impl}##{number}"
Comment on lines +27 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unless issue =~ /.+#\d+/
warn("Invalid issue.\n\n")
puts parser.help()
end
issue = issue.split("#")
number = issue.pop
issue = issue.join().split("/")
impl = issue.pop
options[:issue] = "#{impl}##{number}"
unless issue =~ /([^/]+#\d+)$/
warn("Invalid issue.\n\n")
puts parser.help()
end
options[:issue] = $1

end

parser.on("--missing-issue",
"Find specs that have a todo without a Github issue.") do
options[:missing_issue] = true
end

parser.on("-h", "--help", "Print this help message.") do
puts parser.help()
return nil
end

parser.parse!(args)
options[:dirs] = args.dup.map{ |d| File.expand_path(d) } if args.any?
new(options)
end

def initialize(options)
@options = options
end

##
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably best to follow the doc comment style in the rest of the repo, which doesn't have this leading ##. I think you can omit the @param and @returns as well.

# Given the options provided to the CLI, filters through spec metadata to
# aggregate statistics about the specs. Prints a stats report.
def stats
metadata = get_metadata()

if @options[:todo]
metadata = filter_todos_by_implementation(metadata)
end
Comment on lines +63 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Style nit:

Suggested change
if @options[:todo]
metadata = filter_todos_by_implementation(metadata)
end
metadata = filter_todos_by_implementation(metadata) if @options[:todo]

This will let you write what currently takes 11 lines in 3 without losing much readability 😃.


if @options[:issue]
metadata = filter_todos_by_issue(metadata)
end

if @options[:missing_issue]
metadata = filter_todos_missing_issue(metadata)
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to return metadata as well?


##
# Filter a set of metadata to those with a todo for the given implementation.
#
# @param [Array<SassSpec::TestCaseMetadata>]
# @return Array<SassSpec::TestCaseMetadata>]
def filter_todos_by_implementation(metadata)
metadata.select do |meta|
todos = meta.options[:todo]
next unless todos
todos.select { |todo| todo.include? @options[:todo] }.any?
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use Metadata#todo? here?

end
end

##
# Filter a set of metadata to those with a todo for the given Github issue.
#
# @param [Array<SassSpec::TestCaseMetadata>]
# @return Array<SassSpec::TestCaseMetadata>]
def filter_todos_by_issue(metadata)
metadata.select do |meta|
todos = get_yaml(meta)[:todo]
next unless todos
todos.select { |todo| todo.include? @options[:issue] }.any?
end
end

##
# Filter a set of metadata to those with a todo missing a Github issue.
#
# @param [Array<SassSpec::TestCaseMetadata>]
# @return Array<SassSpec::TestCaseMetadata>]
def filter_todos_missing_issue(metadata)
metadata.select do |meta|
todos = get_yaml(meta)[:todo]
next unless todos
todos.select { |todo| !todo.include? "#" }.any?
end
end

##
# Gets the raw YAML representation of a spec's metadata. Useful when
# inspecting for patterns that get normalized away by TestCaseMetadata.
#
# @param [SassSpec::TestCaseMetadata] metadata
# @return [YAML]
def get_yaml(metadata)
dir = SassSpec::Directory.new(metadata.name)
YAML.load(dir.read("options.yml"))
end
Comment on lines +121 to +124
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than re-reading the metadata here, maybe it would be better to have the Metadata object store the raw options map.


##
# Gets all of the metadata in the spec directories.
#
# @return [Array<SassSpec::TestCaseMetadata>]
def get_metadata
dirs = @options[:dirs] || [SassSpec::SPEC_DIR].map do |dir|
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dirs = @options[:dirs] || [SassSpec::SPEC_DIR].map do |dir|
dirs = (@options[:dirs] || [SassSpec::SPEC_DIR]).map do |dir|

SassSpec::Directory.new(dir)
end

option_files = dirs.flat_map do |dir|
dir.glob("**/options.yml").map do |file|
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point, I think it would be pretty easy to make this find all test cases by making this:

Suggested change
dir.glob("**/options.yml").map do |file|
dir.glob("**/input.{scss,sass,css}").map do |file|

File.join(dir.path, file)
end
end.uniq

metadata = option_files.map do |file|
dir = SassSpec::Directory.new(File.dirname(file))
SassSpec::TestCaseMetadata.new(dir)
end
end
end
26 changes: 26 additions & 0 deletions stats.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env ruby

module SassSpec
module Stats
end
end

require_relative 'lib/sass_spec'
require_relative 'lib/sass_spec/stats'

exceptions = [
OptionParser::InvalidOption,
OptionParser::AmbiguousOption,
OptionParser::MissingArgument,
]

begin
cli = SassSpec::Stats::CLI.parse(ARGV)
cli.stats()
rescue *exceptions => e
warn e.message + "\n\n"
SassSpec::Stats::CLI.parse(%w(-h))
exit 1
rescue
exit 1
end