Flaky tests when using results: :first

[fabioperrella]

Some specs, like this one, calls the method FormatParser.parse with the option results: :first, which is the default value, and checks if the returned parser is the expected one.

In some cases, FormatParser.parse recognizes a file with multiple formats, and using results: :first, it returns only the first one.

The problem is that the order of these results varies.

In this method, there is a logic to sort the parsers by priority, but since there are many with the same priority, the order varies on each execution.

parsers_in_order_of_priority = parsers.to_a.sort do |parser_a, parser_b|
  @parser_priorities[parser_a] <=> @parser_priorities[parser_b  
end

Because of this, some tests fail as the following example:

  1) FormatParser.parse when multiple results are requested without :result=> :all (first result) is expected to eq #<FormatParser::Image:0x00007fa3258d28c0 @format=:dpx, @width_px=1, @height_px=1, @display_width_px=1, @display_height_px=1>
     Failure/Error: it { is_expected.to eq(image) }

       expected: #<FormatParser::Image:0x00007fa3258d28c0 @format=:dpx, @width_px=1, @height_px=1, @display_width_px=1, @display_height_px=1>
            got: #<FormatParser::Audio:0x00007fa321af2488 @format=:mp3, @num_audio_channels=2, @audio_sample_rate_hz=48000, @intrinsics={:id3tags=>[]}, @media_duration_seconds=22.06318816438356>

[fabioperrella]

I don't know exactly why the order sometimes is different, but I could prove it adding this line:

# lib/format_parser.rb
def self.parsers_for(desired_natures, desired_formats, filename_hint = nil)
  #...
  parsers_in_order_of_priority.each { |parser| puts parser.class } # <--- this one
  parsers_in_order_of_priority
end

When I run on my laptop:

irb(main):001:0> file = Tempfile.new
irb(main):002:0> FormatParser.parse(file)
FormatParser::GIFParser
Class
FormatParser::PDFParser
FormatParser::PNGParser
FormatParser::MOOVParser
# ...

When I run in production:

irb(main):005:0> file = Tempfile.new
=> #<Tempfile:/tmp/20201204-5987-1mu9uer>
irb(main):006:0> FormatParser.parse(file)
FormatParser::GIFParser
Class
FormatParser::PNGParser
FormatParser::PDFParser
FormatParser::MOOVParser
# ...

I think it's related to the following MUTEX, which sets a few class variables when registering the parsers:

# lib/format_parser.rb
PARSER_MUX = Mutex.new

#...

def self.register_parser(callable_parser, formats:, natures:, priority: LEAST_PRIORITY)
  parser_provided_formats = Array(formats)
  parser_provided_natures = Array(natures)
  PARSER_MUX.synchronize do
    @parsers ||= Set.new
    @parsers << callable_parser

[linkyndy]

Could it be that it depends on the load order for the Ruby files, which is not defined?

[fabioperrella]

I don't think so. 🤔

I did one more test adding the following line:

def self.register_parser(callable_parser, formats:, natures:, priority: LEAST_PRIORITY)
  puts "register: #{callable_parser.class}"

The results in staging and in my laptop are the same!

[linkyndy]

Maybe rolling the pod will cause the files to be loaded differently within format_parser, and thus you'd see a different order 🤔