diff --git a/.rubocop.yml b/.rubocop.yml index f6418fbe8..7cb663e34 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -33,6 +33,18 @@ Layout/TrailingWhitespace: - spec/cucumber/formatter/pretty_spec.rb - spec/cucumber/formatter/progress_spec.rb +# This file has a couple of very long complex methods which are hard to break down further meaningfully +# -> They also will only ever run one single branch (Due to the fact messages only are ever of one type at a time) +Metrics/CyclomaticComplexity: + Exclude: + - lib/cucumber/repository.rb + +# This file has a couple of very long complex methods which are hard to break down further meaningfully +# -> They also will only ever run one single branch (Due to the fact messages only are ever of one type at a time) +Metrics/PerceivedComplexity: + Exclude: + - lib/cucumber/repository.rb + # Rubocop doesn't like method names in other languages but as Cucumber supports multiple languages, this cop needs to be disabled Naming/AsciiIdentifiers: Enabled: false diff --git a/lib/cucumber/formatter/html.rb b/lib/cucumber/formatter/html.rb index 23229ed9e..e8685d863 100644 --- a/lib/cucumber/formatter/html.rb +++ b/lib/cucumber/formatter/html.rb @@ -7,13 +7,10 @@ module Cucumber module Formatter class HTML < MessageBuilder - include Io - def initialize(config) @io = ensure_io(config.out_stream, config.error_stream) @html_formatter = Cucumber::HTMLFormatter::Formatter.new(@io) @html_formatter.write_pre_message - super(config) end diff --git a/lib/cucumber/formatter/message_builder.rb b/lib/cucumber/formatter/message_builder.rb index 89662dcdc..deffad135 100644 --- a/lib/cucumber/formatter/message_builder.rb +++ b/lib/cucumber/formatter/message_builder.rb @@ -15,6 +15,7 @@ module Cucumber module Formatter class MessageBuilder include Cucumber::Messages::Helpers::TimeConversion + include Io def initialize(config) @config = config @@ -25,6 +26,8 @@ def initialize(config) @step_definitions_by_test_step = Query::StepDefinitionsByTestStep.new(config) @test_case_started_by_test_case = Query::TestCaseStartedByTestCase.new(config) @test_run_started = Query::TestRunStarted.new(config) + @repository = Cucumber::Repository.new + @query = Cucumber::Query.new(@repository) config.on_event :envelope, &method(:on_envelope) config.on_event :gherkin_source_read, &method(:on_gherkin_source_read) @@ -41,9 +44,6 @@ def initialize(config) @current_test_run_started_id = nil @current_test_case_started_id = nil @current_test_step_id = nil - - @repository = Cucumber::Repository.new - @query = Cucumber::Query.new(@repository) end def attach(src, media_type, filename) @@ -87,6 +87,8 @@ def on_gherkin_source_read(event) end def on_test_case_ready(event) + # TODO: Switch this over to using the Repo Query object -> `test_step_by_id` + # TODO: The finder in query is `find_test_step_by` (Using +TestStepStarted+ message) event.test_case.test_steps.each do |step| @test_case_by_step_id[step.id] = event.test_case end @@ -100,6 +102,9 @@ def on_test_case_ready(event) ) ) + # TODO: Once we're comfortable switching this over. Call @repository.update(message) alongside output_envelope + # however this may not be necessary as output_envelope may/should already be doing this? + output_envelope(message) end @@ -108,6 +113,8 @@ def test_step_to_message(step) Cucumber::Messages::TestStep.new( id: step.id, + # TODO: This "fake query" is only used once. It can likely be replace by `find_pickle_step_by` which + # takes a +TestStep+ message from the repo directly. pickle_step_id: @pickle_step_by_test_step.pickle_step_id(step), step_definition_ids: @step_definitions_by_test_step.step_definition_ids(step), step_match_arguments_lists: step_match_arguments_lists(step) @@ -152,6 +159,7 @@ def parameter_type_name(step_match_argument) end def on_test_run_started(*) + # TODO: Switch this over to using the Query object -> `find_test_run_started` @current_test_run_started_id = @test_run_started.id message = Cucumber::Messages::Envelope.new( diff --git a/lib/cucumber/formatter/rerun.rb b/lib/cucumber/formatter/rerun.rb index 5ee5080c4..5cb424214 100644 --- a/lib/cucumber/formatter/rerun.rb +++ b/lib/cucumber/formatter/rerun.rb @@ -1,52 +1,81 @@ # frozen_string_literal: true require 'cucumber/formatter/io' +require 'cucumber/formatter/message_builder' module Cucumber module Formatter - class Rerun - include Formatter::Io - + class Rerun < MessageBuilder def initialize(config) @io = ensure_io(config.out_stream, config.error_stream) - @config = config - @failures = {} - config.on_event :test_case_finished do |event| - test_case, result = *event.attributes - if @config.strict.strict?(:flaky) - next if result.ok?(strict: @config.strict) - - add_to_failures(test_case) - else - unless @latest_failed_test_case.nil? - if @latest_failed_test_case != test_case - add_to_failures(@latest_failed_test_case) - @latest_failed_test_case = nil - elsif result.ok?(strict: @config.strict) - @latest_failed_test_case = nil - end - end - @latest_failed_test_case = test_case unless result.ok?(strict: @config.strict) + super(config) + end + + def output_envelope(envelope) + @repository.update(envelope) + finish_report if envelope.test_run_finished + end + + private + + def finish_report + @query.find_all_test_case_started.each do |test_case| + status = @query.find_most_severe_test_step_result_by(test_case).status + # RULE: Don't log test cases without a pickle (Unsure what these could be?) + pickle = @query.find_pickle_by(test_case) + next if pickle.nil? + + # RULE: (Configuration specific) + # -> If the test case has already been logged (And so we're retrying), we remove prior references of failures + if passing?(test_case) && !rerun_flaky_tests? + uri_and_location_hash[pickle.uri].delete(pickle.location.line) + next end + + # RULE: (Configuration specific - to be amended once CCK conformance is finalised) + # -> If the strict configuration permits the result - handle it accordingly + next if status == 'UNDEFINED' && !@config.strict.strict?(:undefined) + next if status == 'PENDING' && !@config.strict.strict?(:pending) + + # RULE: Passing test cases are not considered failures (Don't log these) + next if passing?(test_case) + + # RULE: Skipped test cases are not considered failures (on their own, don't log these) + next if skipped?(test_case) + + # RULE: Before logging a failure, ensure we are not on a retried test case (Don't log a retry multiple times) + next if test_case.attempt > 1 + + # Log the failure if every other skip rule has not been met, and the failure has not already been logged + uri_and_location_hash[pickle.uri] << pickle.location.line end - config.on_event :test_run_finished do - add_to_failures(@latest_failed_test_case) unless @latest_failed_test_case.nil? - next if @failures.empty? - @io.print file_failures.join("\n") + # Generate the final output from the logged failures to be formatted in the io output + @io.print(failure_array.join("\n")) + end + + def failure_array + uri_and_location_hash.filter_map do |uri, lines| + "#{uri}:#{lines.join(':')}" if lines.any? end end - private + def uri_and_location_hash + @uri_and_location_hash ||= Hash.new { |hash, key| hash[key] = Set.new } + end + + def rerun_flaky_tests? + @config.strict.strict?(:flaky) + end - def file_failures - @failures.map { |file, lines| [file, lines].join(':') } + def passing?(test_case_started) + most_severe_test_step_result = @query.find_most_severe_test_step_result_by(test_case_started) + most_severe_test_step_result.status == Cucumber::Messages::TestStepResultStatus::PASSED end - def add_to_failures(test_case) - location = test_case.location - @failures[location.file] ||= [] - @failures[location.file] << location.lines.max unless @failures[location.file].include?(location.lines.max) + def skipped?(test_case_started) + most_severe_test_step_result = @query.find_most_severe_test_step_result_by(test_case_started) + most_severe_test_step_result.status == Cucumber::Messages::TestStepResultStatus::SKIPPED end end end diff --git a/lib/cucumber/query.rb b/lib/cucumber/query.rb index 416040678..735735197 100644 --- a/lib/cucumber/query.rb +++ b/lib/cucumber/query.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'cucumber/repository' +require 'cucumber/messages' # Given one Cucumber Message, find another. # @@ -12,6 +13,9 @@ class Query attr_reader :repository private :repository + include Cucumber::Messages::Helpers::TimeConversion + include Cucumber::Messages::Helpers::TestStepResultComparator + def initialize(repository) @repository = repository end @@ -23,31 +27,15 @@ def initialize(repository) # Missing: findAllUndefinedParameterTypes # TODO: find****By methods (16/25) Complete - # Missing: findLocationOf (1 variant) - This strictly speaking isn't a findBy but is located within them # Missing: findSuggestionsBy (2 variants) # Missing: findUnambiguousStepDefinitionBy (1 variant) # Missing: findTestStepFinishedAndTestStepBy (1 variant) - # Missing: findMostSevereTestStepResultBy (2 variants) # Missing: findAttachmentsBy (2 variants) # Missing: findTestCaseDurationBy (2 variant) - # Missing: findLineageBy (9 variants!) + # REDUNDANT: findLineageBy (9 variants!) + # REDUNDANT: findLocationOf (1 variant) - This strictly speaking isn't a findBy but is located within them + # To Review: findMostSevereTestStepResultBy (2 variants) # To Review: findTestRunDuration (1 variant) - This strictly speaking isn't a findBy but is located within them - # Complete: findMeta (1 variant) - # Complete: findTestRunHookStartedBy (1 variant) - # Complete: findTestRunHookFinishedBy (1 variant) - # Complete: findPickleStepBy (1 variant) - # Complete: findStepDefinitionsBy (1 variant) - # Complete: findStepBy (1 variant) - # Complete: findTestRunFinished (1 variant) - # Complete: findTestRunStarted (1 variant) - # Fully Complete (2/2): findTestStepsStartedBy (2 variants) - # Fully Complete (2/2): findTestStepBy (2 variants) - # Fully Complete (3/3): findTestCaseStartedBy (3 variants) - # Fully Complete (1/1): findTestCaseFinishedBy (1 variant) - # Fully Complete (4/4): findTestCaseBy (4 variants) - # Fully Complete (5/5): findPickleBy (5 variants) - # Fully Complete (3/3): findHookBy (3 variants) - # Fully Complete (2/2): findTestStepsFinishedBy (2 variants) def count_test_cases_started find_all_test_case_started.length @@ -121,6 +109,23 @@ def find_meta repository.meta end + # This method will be called with 1 of these 2 messages + # [TestCaseStarted || TestCaseFinished] + def find_most_severe_test_step_result_by(message) + ensure_only_message_types!(message, %i[test_case_started test_case_finished], '#find_most_severe_test_step_result_by') + + if message.is_a?(Cucumber::Messages::TestCaseStarted) + find_test_steps_finished_by(message) + .map(&:test_step_result) + .max_by { |test_step_result| test_step_result_rankings[test_step_result.status] } + # Java code: "PREVIOUS".max(comparing(TestStepResult::getStatus, new TestStepResultStatusComparator())); + else + test_case_started_message = find_test_case_started_by(message) + test_case_started_message && find_most_severe_test_step_result_by(test_case_started_message) + # Java code: return findTestCaseStartedBy(testCaseFinished).flatMap(this::findMostSevereTestStepResultBy); + end + end + # This method will be called with 1 of these 5 messages # [TestCase || TestCaseStarted || TestCaseFinished || TestStepStarted || TestStepFinished] def find_pickle_by(message) @@ -235,7 +240,7 @@ def find_test_steps_finished_by(message) ensure_only_message_types!(message, %i[test_case_started test_case_finished], '#find_test_steps_finished_by') if message.is_a?(Cucumber::Messages::TestCaseStarted) - test_steps_finished_by_test_case_started_id.fetch(message.id, []) + repository.test_steps_finished_by_test_case_started_id.fetch(message.id, []) else test_case_started_message = find_test_case_started_by(message) test_case_started_message.nil? ? [] : find_test_steps_finished_by(test_case_started_message) diff --git a/lib/cucumber/repository.rb b/lib/cucumber/repository.rb index 82668d33a..c5d8fca65 100644 --- a/lib/cucumber/repository.rb +++ b/lib/cucumber/repository.rb @@ -12,8 +12,7 @@ class Repository :test_run_hook_started_by_id, :test_run_hook_finished_by_test_run_hook_started_id, :test_step_by_id, :test_steps_started_by_test_case_started_id, :test_steps_finished_by_test_case_started_id - # TODO: Missing structs (3) - # final Map lineageById = new HashMap<>(); + # TODO: Missing structs (2) # final Map> suggestionsByPickleStepId = new LinkedHashMap<>(); # final List undefinedParameterTypes = new ArrayList<>(); diff --git a/spec/cucumber/formatter/rerun_spec.rb b/spec/cucumber/formatter/rerun_spec.rb index 69b3001f6..54a724043 100644 --- a/spec/cucumber/formatter/rerun_spec.rb +++ b/spec/cucumber/formatter/rerun_spec.rb @@ -36,7 +36,7 @@ module Formatter end described_class.new(config) - execute [gherkin], [StandardStepActions.new], config.event_bus + execute [gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus config.event_bus.test_run_finished expect(io.string).to eq 'foo.feature:3:6' @@ -70,7 +70,7 @@ module Formatter end described_class.new(config) - execute [foo, bar], [StandardStepActions.new], config.event_bus + execute [foo, bar], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus config.event_bus.test_run_finished expect(io.string).to eq "foo.feature:3:6\nbar.feature:3" @@ -88,14 +88,14 @@ module Formatter end described_class.new(config) - execute [gherkin], [StandardStepActions.new], config.event_bus + execute [gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus config.event_bus.test_run_finished - expect(io.string).to eq '' + expect(io.string).to eq('') end end - context 'with only a flaky scenarios' do + context 'with a flaky scenario' do context 'with option --no-strict-flaky' do it 'prints nothing' do gherkin = gherkin('foo.feature') do @@ -107,10 +107,11 @@ module Formatter end described_class.new(config) - execute [gherkin], [FakeObjects::FlakyStepActions.new], config.event_bus + execute [gherkin], [FakeObjects::FlakyStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus + config.event_bus.test_run_finished - expect(io.string).to eq '' + expect(io.string).to eq('') end end @@ -118,7 +119,7 @@ module Formatter let(:config) { Configuration.new(out_stream: io, strict: Core::Test::Result::StrictConfiguration.new([:flaky])) } it 'prints the location of the flaky scenario' do - foo = gherkin('foo.feature') do + gherkin = gherkin('foo.feature') do feature do scenario do step 'flaky' @@ -127,14 +128,14 @@ module Formatter end described_class.new(config) - execute [foo], [FakeObjects::FlakyStepActions.new], config.event_bus + execute [gherkin], [FakeObjects::FlakyStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus config.event_bus.test_run_finished - expect(io.string).to eq 'foo.feature:3' + expect(io.string).to eq('foo.feature:3') end - it 'does not include retried failing scenarios more than once' do - foo = gherkin('foo.feature') do + it 'does not include the same failing scenario more than once' do + gherkin = gherkin('foo.feature') do feature do scenario do step 'failing' @@ -143,10 +144,10 @@ module Formatter end described_class.new(config) - execute [foo, foo], [StandardStepActions.new], config.event_bus + execute [gherkin, gherkin], [StandardStepActions.new, Filters::BroadcastTestRunStartedEvent.new(config), Filters::BroadcastTestCaseReadyEvent.new(config)], config.event_bus config.event_bus.test_run_finished - expect(io.string).to eq 'foo.feature:3' + expect(io.string).to eq('foo.feature:3') end end end diff --git a/spec/support/fake_objects.rb b/spec/support/fake_objects.rb index a37b26074..3baa49583 100644 --- a/spec/support/fake_objects.rb +++ b/spec/support/fake_objects.rb @@ -47,6 +47,7 @@ def test_case(test_case) step.with_action {} end + test_case.with_steps(failing_test_steps).describe_to(receiver) test_case.with_steps(failing_test_steps).describe_to(receiver) test_case.with_steps(passing_test_steps).describe_to(receiver) end