RoF

[3v0k4]

Description

See https://github.com/KnapsackPro/knapsack-pro-api/pull/1455

Checks

• I added the changes to the UNRELEASED section of the CHANGELOG.md, including the needed bump (i.e., patch, minor, major)
• I followed the architecture outlined below for RSpec in Queue Mode:
  • Pure: lib/knapsack_pro/pure/queue/rspec_pure.rb contains pure functions that are unit tested.
  • Extension: lib/knapsack_pro/extensions/rspec_extension.rb encapsulates calls to RSpec internals and is integration and E2E tested.
  • Runner: lib/knapsack_pro/runners/queue/rspec_runner.rb invokes the pure code and the extension to produce side effects, which are integration and E2E tested.

[ArturT]

Great work!

[ArturT]

question

What's the purpose of this method here? Is it to avoid using the fixed_queue_split method that could print logger info a few times when KNAPSACK_PRO_FIXED_QUEUE_SPLIT is not set? If @fixed_queue_split ||= false then it would evaluate the begin block each time you call the fixed_queue_split method.

Perhaps changing def fixed_queue_split to the following would be enough and then we don't need fixed_queue_split_?:

def fixed_queue_split
  return @fixed_queue_split if defined?(@fixed_queue_split)

  @fixed_queue_split = begin
    env_name = 'KNAPSACK_PRO_FIXED_QUEUE_SPLIT'
    computed = env_for(env_name, :fixed_queue_split).to_s

    if !ENV.key?(env_name)
      KnapsackPro.logger.info("#{env_name} is not set. Using default value: #{computed}. Learn more at #{KnapsackPro::Urls::FIXED_QUEUE_SPLIT}")
    end

    computed
  end
end

[ArturT]

suggestion

Why is the test queue empty? Let's make it clear to the user.

Suggested change

	KnapsackPro.logger.info("No tests were executed because the test queue is empty.")
	KnapsackPro.logger.info("No tests were executed because the test queue has already been consumed.")

[ArturT]

Perhaps comments could be removed here and in L70?