rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.23k stars 765 forks source link

Bring formatters_spec in line with changes to formatters #2887

Closed effleurager closed 3 years ago

effleurager commented 3 years ago

Back in 2014, the legacy formatter deprecation message was changed, but that change was never carried over to the spec file.

This PR is mostly driven by another PR, which has one test failing.

pirj commented 3 years ago

Thanks! Would you like to send a similar PR to 4-0-dev branch? The same spec was updated there.

JonRowe commented 3 years ago

Thanks for taking the time to contribute here, but this spec matches the message, it didn't need changing, its merely important that it outputs that it is deprecated not the exact message content. Changing this to make it more specific merely makes the spec more brittle.

effleurager commented 3 years ago

Would you prefer I create another PR with a more general approach, and for 4-0-dev?

effleurager commented 3 years ago

this spec matches the message, it didn't need changing

From main, this test currently fails

Failures:

  1) RSpec::Core::Formatters::Loader#add(formatter) when a legacy formatter is added without RSpec::LegacyFormatters issues a deprecation
     Failure/Error:
       expect_warn_deprecation(
         /The #{formatter_class} formatter uses the deprecated formatter interface.+#{__FILE__}:#{__LINE__ + 1}/
       )

       #<RSpec::Core::Reporter:0x000056406de48118 @configuration=#<RSpec::Core::Configuration:0x000056406de45490 @start_time=2021-04-22 16:41:40.84744101 +0000, @expectation_frameworks=[], @include_modules=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de45418 @applies_predicate=:any?, @items_and_filters=[], @applicable_keys=#<RSpec::Core::Set:0x000056406de453a0 @values={}>, @proc_keys=#<RSpec::Core::Set:0x000056406de451c0 @values={}>, @memoized_lookups={}>, @extend_modules=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de44f18 @applies_predicate=:any?, @items_and_filters=[], @applicable_keys=#<RSpec::Core::Set:0x000056406de44e78 @values={}>, @proc_keys=#<RSpec::Core::Set:0x000056406de44cc0 @values={}>, @memoized_lookups={}>, @prepend_modules=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de44b58 @applies_predicate=:any?, @items_and_filters=[], @applicable_keys=#<RSpec::Core::Set:0x000056406de44a18 @values={}>, @proc_keys=#<RSpec::Core::Set:0x000056406de44900 @values={}>, @memoized_lookups={}>, @bisect_runner=:fork, @bisect_runner_class=nil, @before_suite_hooks=[], @after_suite_hooks=[], @mock_framework=nil, @files_or_directories_to_run=[], @loaded_spec_files=#<RSpec::Core::Set:0x000056406de445e0 @values={}>, @color=false, @color_mode=:off, @pattern="**{,/*/**}/*_spec.rb", @exclude_pattern="", @failure_exit_code=1, @error_exit_code=nil, @fail_if_no_examples=false, @spec_files_loaded=false, @backtrace_formatter=#<RSpec::Core::BacktraceFormatter:0x000056406de44428 @full_backtrace=false, @exclusion_patterns=[/(?-mix:(?-mix:\/lib\/rspec\/(core|mocks|expectations|support|matchers|rails|autorun)(\.rb|\/))|rubygems\/core_ext\/kernel_require\.rb)|(?-mix:\/lib\d*\/ruby\/)|(?-mix:bin\/)|(?-mix:exe\/rspec)|(?-mix:\/lib\/bundler\/)|(?-mix:\/exe\/bundle:)/], @inclusion_patterns=[]>, @default_path="spec", @project_source_dirs=["spec", "lib", "app"], @deprecation_stream=#<IO:<STDOUT>>, @output_stream=#<IO:<STDOUT>>, @reporter=#<RSpec::Core::Reporter:0x000056406de48118 ...>, @reporter_buffer=nil, @filter_manager=#<RSpec::Core::FilterManager:0x000056406de4aaf8 @exclusions=#<RSpec::Core::FilterRules:0x000056406de4aa30 @rules={}, @opposite=#<RSpec::Core::InclusionRules:0x000056406de4a9b8 @rules={}, @opposite=#<RSpec::Core::FilterRules:0x000056406de4aa30 ...>>>, @inclusions=#<RSpec::Core::InclusionRules:0x000056406de4a9b8 @rules={}, @opposite=#<RSpec::Core::FilterRules:0x000056406de4aa30 @rules={}, @opposite=#<RSpec::Core::InclusionRules:0x000056406de4a9b8 ...>>>>, @static_config_filter_manager=#<RSpec::Core::FilterManager:0x000056406de4a918 @exclusions=#<RSpec::Core::FilterRules:0x000056406de4a7d8 @rules={}, @opposite=#<RSpec::Core::InclusionRules:0x000056406de4a760 @rules={}, @opposite=#<RSpec::Core::FilterRules:0x000056406de4a7d8 ...>>>, @inclusions=#<RSpec::Core::InclusionRules:0x000056406de4a760 @rules={}, @opposite=#<RSpec::Core::FilterRules:0x000056406de4a7d8 @rules={}, @opposite=#<RSpec::Core::InclusionRules:0x000056406de4a760 ...>>>>, @ordering_manager=#<RSpec::Core::Ordering::ConfigurationManager:0x000056406de4a698 @ordering_registry=#<RSpec::Core::Ordering::Registry:0x000056406de4a670 @configuration=#<RSpec::Core::Ordering::ConfigurationManager:0x000056406de4a698 ...>, @strategies={:random=>#<RSpec::Core::Ordering::Random:0x000056406de4a558 @configuration=#<RSpec::Core::Ordering::ConfigurationManager:0x000056406de4a698 ...>, @used=false>, :recently_modified=>#<RSpec::Core::Ordering::RecentlyModified:0x000056406de4a530>, :defined=>#<RSpec::Core::Ordering::Identity:0x000056406de4a418>, :global=>#<RSpec::Core::Ordering::Identity:0x000056406de4a418>}>, @seed=43176, @seed_forced=false, @order_forced=false>, @preferred_options={}, @failure_color=:red, @success_color=:green, @pending_color=:yellow, @default_color=:white, @fixed_color=:blue, @detail_color=:cyan, @profile_examples=false, @requires=[], @libs=[], @derived_metadata_blocks=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de4a0f8 @applies_predicate=:any?, @items_and_filters=[], @applicable_keys=#<RSpec::Core::Set:0x000056406de49fe0 @values={}>, @proc_keys=#<RSpec::Core::Set:0x000056406de49ef0 @values={}>, @memoized_lookups={}>, @threadsafe=true, @max_displayed_failure_line_count=10, @world=#<RSpec::Core::World:0x000056406de49478 @wants_to_quit=false, @configuration=#<RSpec::Core::Configuration:0x000056406de45490 ...>, @example_groups=[], @example_group_counts_by_spec_file={}, @filtered_examples={}>, @shared_context_metadata_behavior=:trigger_inclusion, @hooks=#<RSpec::Core::Hooks::HookCollections:0x000056406de49ba8 @owner=#<RSpec::Core::Configuration:0x000056406de45490 ...>, @filterable_item_repo_class=RSpec::Core::FilterableItemRepository::QueryOptimized, @before_example_hooks=nil, @after_example_hooks=nil, @before_context_hooks=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de48de8 @applies_predicate=:all?, @items_and_filters=[[#<struct RSpec::Core::Hooks::BeforeHook block=#<Proc:0x000056406de49158 /home/circleci/project/sample_code/rspec/rspec-core/spec/support/sandboxing.rb:11>, options={}>, {}]], @applicable_keys=#<RSpec::Core::Set:0x000056406de48d70 @values={}>, @proc_keys=#<RSpec::Core::Set:0x000056406de48c58 @values={}>, @memoized_lookups={}>, @after_context_hooks=nil, @around_example_hooks=#<RSpec::Core::FilterableItemRepository::QueryOptimized:0x000056406de499a0 @applies_predicate=:all?, @items_and_filters=[[#<struct RSpec::Core::Hooks::AroundHook block=#<Proc:0x000056406de49cc0 /home/circleci/project/sample_code/rspec/rspec-core/lib/rspec/core/configuration.rb:2245>, options={:aggregate_failures=>true}>, {:aggregate_failures=>true}]], @applicable_keys=#<RSpec::Core::Set:0x000056406de49950 @values={:aggregate_failures=>true}>, @proc_keys=#<RSpec::Core::Set:0x000056406de49810 @values={}>, @memoized_lookups={}>>, @output_wrapper=#<IO:<STDOUT>>, @formatter_loader=#<RSpec::Core::Formatters::Loader:0x000056406de4fe68 @formatters=[], @reporter=#<RSpec::Core::Reporter:0x000056406de48118 ...>, @default_formatter="progress">>, @listeners={}, @examples=[], @failed_examples=[], @pending_examples=[], @load_time=nil, @start=nil, @duration=nil, @non_example_exception_count=0, @setup_default=#<Proc:0x000056406de4fc10 /home/circleci/project/sample_code/rspec/rspec-core/lib/rspec/core/reporter.rb:47 (lambda)>, @setup=false, @profiler=nil> received :deprecation with unexpected arguments
         expected: (include {:message => (match /The #<Class:0x000056406bf7b898> formatter uses the deprecated formatter interface.+\/home\/circleci\/project\/sample_code\/rspec\/rspec-core\/spec\/rspec\/core\/formatters_spec.rb:87/)})
              got: ({:message=>" The #<Class:0x000056406bf7b898> formatter uses the deprecated formatter interface not supported directly by RSpec 3.  To continue to use this formatter you must install the `rspec-legacy_formatters` gem, which provides support for legacy formatters or upgrade the formatter to a compatible version.  Formatter added at: /home/circleci/project/sample_code/rspec/rspec-core/spec/rspec/core/formatters_spec.rb:88:in `block (4 levels) in <module:Formatters>'\n"})
       Diff:
       @@ -1,2 +1,3 @@
       -["include {:message => (match /The #<Class:0x000056406bf7b898> formatter uses the deprecated formatter interface.+\\/home\\/circleci\\/project\\/sample_code\\/rspec\\/rspec-core\\/spec\\/rspec\\/core\\/formatters_spec.rb:87/)}"]
       +[{:message=>
       +   " The #<Class:0x000056406bf7b898> formatter uses the deprecated formatter interface not supported directly by RSpec 3.  To continue to use this formatter you must install the `rspec-legacy_formatters` gem, which provides support for legacy formatters or upgrade the formatter to a compatible version.  Formatter added at: /home/circleci/project/sample_code/rspec/rspec-core/spec/rspec/core/formatters_spec.rb:88:in `block (4 levels) in <module:Formatters>'\n"}]
     # ./spec/rspec/core/formatters_spec.rb:85:in `block (4 levels) in <module:Formatters>'
     # ./spec/support/sandboxing.rb:16:in `block (3 levels) in <top (required)>'
     # ./spec/support/sandboxing.rb:7:in `block (2 levels) in <top (required)>'

Finished in 12.65 seconds (files took 0.55625 seconds to load)
1968 examples, 1 failure, 1 pending
JonRowe commented 3 years ago
-\\/home\\/circleci\\/project\\/sample_code\\/rspec\\/rspec-core\\/spec\\/rspec\\/core\\/formatters_spec.rb:87/)}"]
+/home/circleci/project/sample_code/rspec/rspec-core/spec/rspec/core/formatters_spec.rb:88

Somme how your build is changing the line number. It passes on our CI

JonRowe commented 3 years ago

If you wish to improve the line number detection without adding all of the message I will look at it again, but the whole point of this spec is just to check the deprecation is issued, not the exact message. (Except that it should check an exact line number, because thats the point of our deprecation code).

effleurager commented 3 years ago
-\\/home\\/circleci\\/project\\/sample_code\\/rspec\\/rspec-core\\/spec\\/rspec\\/core\\/formatters_spec.rb:87/)}"]
+/home/circleci/project/sample_code/rspec/rspec-core/spec/rspec/core/formatters_spec.rb:88

Somme how your build is changing the line number. It passes on our CI

My bad, I was programming too late into the morning 🙈

So part of rufo's test suite is to test other codebases and see that they still test correctly before and after formatting. The issue comes in with the after bit: image

Moving that bracket down to the next line means the __LINE__ + 1 rule doesn't work. But I'll definitely try to create a more flexible line number regex 👍🏽

JonRowe commented 3 years ago

Ah I see, so yes you change our code you break our specs, this is to be expected as we do care about our line numbers, I'm afraid I don't want a flexible regex on the line number, it needs to match the line the code actually uses. Perhaps you can apply a transformation of expected changed required to the specs in your build?

JonRowe commented 3 years ago

As an aside, note our configured max line length is 130, rufo supports configuring that from memory, perhaps make sure its configured to that for us?

effleurager commented 3 years ago

Ah I see, so yes you change our code you break our specs, this is to be expected as we do care about our line numbers, I'm afraid I don't want a flexible regex on the line number, it needs to match the line the code actually uses. Perhaps you can apply a transformation of expected changed required to the specs in your build?

We do currently have a list of exclusions per repository tested, so I could add that file as an exclusion for rspec. But I find it more interesting that it passes on ruby < 2.6.3 and fails 2.6.3 and above. That's something I'll investigate 🔍

effleurager commented 3 years ago

As an aside, note our configured max line length is 130, rufo supports configuring that from memory, perhaps make sure its configured to that for us?

Rufo doesn't yet support line length configuration, and does not parse rubocop configs.