Single file skip parallel

[WJWH]

This MR makes it so that if there is only a single file to be inspected, we do not split off extra worker processes to do the work. This is because doing the work in a separate process can never be faster than doing it inline. I benchmarked this change on my dev machine (with 6 physical cores) and it saves 40-50 ms per run (only if only a single file is inspected of course, otherwise it will take as long as before).

I did not know what to update the test with, so I just made it expect the new text. Let me know if this should be different.

(This is a fixed version of #10889, where I had messed up the rebasing)

---

Before submitting the PR make sure the following are checked:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Updated tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

[bbatsov]

Btw, shouldn't this be the first line of the method?

[WJWH]

If we make this the first line of the method then the ensure clause needs to have a separate begin block, otherwise it will replace @options with nil. I opted for this way because it looked slightly cleaner to my eyes and needed less changes.

Would you like me to rewrite it to skip the .dup?

[bbatsov]

I see. I didn't notice the ensure. Probably it would have been nicer to have here if/else instead of if + return, although I assume RuboCop forced you to write the code like this.

[WJWH]

The following passes tests and rubocop inspection:

    # Warms up the RuboCop cache by forking a suitable number of RuboCop
    # instances that each inspects its allotted group of files.
    def warm_cache(target_files)
      if target_files.length <= 1
        puts 'Skipping parallel inspection: only a single file needs inspection' if @options[:debug]
      else
        begin
          saved_options = @options.dup
          puts 'Running parallel inspection' if @options[:debug]
          %i[autocorrect safe_autocorrect].each { |opt| @options[opt] = false }
          Parallel.each(target_files) { |target_file| file_offenses(target_file) }
        ensure
          @options = saved_options
        end
      end
    end

[dvandersluis]

Can you take a look at this test failure:

1) RuboCop::CLI options when supporting server --server-status when server is runnning displays server status
     Failure/Error: expect(output).to match(/RuboCop server \(\d+\) is running./)
     
       expected "RuboCop server is not running.\n" to match /RuboCop server \(\d+\) is running./
       Diff:
       @@ -1 +1 @@
       -/RuboCop server \(\d+\) is running./
       +RuboCop server is not running.
       
     # ./spec/rubocop/cli/options_spec.rb:189:in `block (5 levels) in <top (required)>'

it's different than the flaky test failure from #10897.

[WJWH]

@dvandersluis I looked at the failure but it does not look like it should be caused by the changes in this MR. I tried reproducing the error on my machine but cannot get it to fail the test suite even with many test queue workers. 😐

[bbatsov]

Can you rebase on top of master?

[bbatsov]

@koic Any idea why the Windows jobs are failing? This seems unrelated to the changes.

[koic]

This looks like an expected failure on Windows CI.

Failures:

  1) RuboCop::CLI options --parallel on Windows prints a warning
     Failure/Error: expect($stderr.string).to include('Process.fork is not supported by this Ruby')
       expected "" to include "Process.fork is not supported by this Ruby"
     # ./spec/rubocop/cli/options_spec.rb:19:in `block (4 levels) in <top (required)>'
     # ./spec/support/cli_spec_behavior.rb:26:in `block (2 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (4 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `chdir'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (3 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:7:in `block (2 levels) in <top (required)>'
     # tasks/spec_runner.rake:40:in `block in run_specs'
     # tasks/spec_runner.rake:52:in `with_encoding'
     # tasks/spec_runner.rake:36:in `run_specs'
     # tasks/spec_runner.rake:165:in `block in <top (required)>'

Finished in 2 minutes 55.7 seconds (files took 4.16 seconds to load)
18732 examples, 1 failure, 15 pending

Failed examples:

rspec ./spec/rubocop/cli/options_spec.rb:17 # RuboCop::CLI options --parallel on Windows prints a warning

https://github.com/rubocop/rubocop/runs/7945357268

The message Process.fork is not supported by this Ruby is printed by Parallel gem.
https://github.com/grosser/parallel/blob/v1.22.1/lib/parallel.rb#L275

Probably this message is no longer printed because the test only has one inspection target file.

[WJWH]

I'll have a look at it this weekend.

[koic]

Can you update the PR number? 😅

Suggested change

	* [#1003](https://github.com/rubocop/rubocop/pull/10903): Skip forking off extra processes for parallel inspection when only a single file needs to be inspected. ([@wjwh][])
	* [#10903](https://github.com/rubocop/rubocop/pull/10903): Skip forking off extra processes for parallel inspection when only a single file needs to be inspected. ([@wjwh][])

[koic]

I commented on the changelog entry, anyway overall it looks good. Can you squash your commits into one?

[bbatsov]

It took us a while, but we made it to the finish line. :-) Thanks!

[WJWH]

Thanks for the help and guidance!