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

Fixes the case, when config_store is nil #24

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
13 changes: 12 additions & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ require "bundler/gem_tasks"
require "rake/testtask"
require "rubocop/rake_task"

Rake::TestTask.new(:test) do |t|
Rake::TestTask.new(:rubocop_md_tests) do |t|
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why prefixing with rubocop_md? This is the Rakefile for the project, so theme is implied.

t.libs << "test"
t.libs << "lib"
t.warning = false
Expand All @@ -11,4 +11,15 @@ end

RuboCop::RakeTask.new

task :test do
ENV["MD_LOAD_MODE"] = "inline"
$stdout.puts "⚙️ Runs rubocop with '-r rubocop_md' options"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding notifications is good idea, but no in the Rakefile. It should be a part of the test suite, so it doesn't depend on the way we run tests (via Rake, via Ruby, or whatever). Having notification here can result in false positives when the actual test suite is not using this env var.

Rake::Task[:rubocop_md_tests].invoke

ENV["MD_LOAD_MODE"] = "config"
$stdout.puts "⚙️ Runs rubocop with 'required rubocop_md' section in .rubocop.yml"
Rake::Task[:rubocop_md_tests].reenable
Rake::Task[:rubocop_md_tests].invoke
Comment on lines +21 to +22
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should define individual tasks and then an aggregate using Rake dependencies.

The main reason is that we need a way to run each mode individually.

end

task default: [:rubocop, :test]
5 changes: 3 additions & 2 deletions lib/rubocop/markdown/rubocop_ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ def markdown_file?(file)

RuboCop::Runner.prepend(Module.new do
# Set config store for Markdown
def initialize(*args)
super
def get_processed_source(*args)
RuboCop::Markdown.config_store = @config_store

super
end

# Do not cache markdown files, 'cause cache doesn't know about processing.
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/.rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
inherit_from:
- "../../.rubocop.yml"

require:
- "rubocop-md"
2 changes: 1 addition & 1 deletion test/fixtures/configs/no_autodetect.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
inherit_from: "../../../.rubocop.yml"
inherit_from: "../.rubocop.yml"

Markdown:
Autodetect: false
3 changes: 2 additions & 1 deletion test/fixtures/configs/no_warn_invalid.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
inherit_from: "../../../.rubocop.yml"
inherit_from: "../.rubocop.yml"

Markdown:
WarnInvalid: false

29 changes: 27 additions & 2 deletions test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,31 @@
require "fileutils"

module RuboCopRunner
MD_LOAD_INLINE_MODE = "inline"

def run_rubocop(path, options: "")
md_path = File.expand_path("../lib/rubocop-md.rb", __dir__)
output, _status = Open3.capture2(
"bundle exec rubocop -r #{md_path} #{options} #{path}",
cmd_command_by_env(path, options),
chdir: File.join(__dir__, "fixtures")
)

output
end

private

def cmd_command_by_env(path, options)
cmd_command = "bundle exec rubocop"
load_mode = ENV.fetch("MD_LOAD_MODE", MD_LOAD_INLINE_MODE)

if load_mode == MD_LOAD_INLINE_MODE
md_path = File.expand_path("../lib/rubocop-md.rb", __dir__)

cmd_command = "#{cmd_command} -r #{md_path}"
end

"#{cmd_command} #{options} #{path}"
end
end
Comment on lines +21 to 33
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an example of improper usage of the extract method pattern. It breaks cohesion; previously, the code related to the command generation was in a single place; now, we have it spread across the file and we lose the context (what is -r, where it comes from)?

Longer methods are sometimes (quite often) better.


class RuboCop::Markdown::AnalyzeTest < Minitest::Test
Expand All @@ -26,6 +43,14 @@ def test_single_snippet_file
assert_match %r{Style/StringLiterals}, res
end

def test_file_with_format_options
res = run_rubocop("single_snippet.md", options: "--format progress")

assert_match %r{Inspecting 1 file}, res
assert_match %r{1 offense detected}, res
assert_match %r{Style/StringLiterals}, res
end

def test_multiple_snippets_file
res = run_rubocop("multiple_snippets.markdown")

Expand Down