user4165455
user4165455

Reputation:

RSpec failing under unknown (seemingly specific) circumstances

I'm relatively new to ruby, moving into it from languages like Python, and C# so please forgive me if this is an obvious bug I'm missing.

The Issue

I was writing code for a tool to help myself and other developers generate .editorconfig files as a way to learn Ruby. I use rspec as my testing framework and wrote the following tests:

module EditorConfigGenerator

  RSpec.describe EditorConfigGenerator::FileGenerator do
    configs_sensible_defaults = [EditorConfigGenerator::EditorConfig.new({root: true, indent_style: 'space', indent_size: 2,
      end_of_line: 'lf', charset: 'utf-8',
    trim_trailing_whitespace: true, insert_final_newline: true})]
    file_generator = EditorConfigGenerator::FileGenerator.new(configs_sensible_defaults)

    context "A collection of one EditorConfig object is provided" do
      it "displays a preview of the file output" do
        # Newlines are automatically inserted into ruby multi-line strings
        output = "root = true
                [*]
                indent_style = space
                indent_size = 2
                end_of_line = lf
                charset = utf-8
                trim_trailing_whitespace = true
                insert_final_newline = true
      "
        # String.delete(' ')  to remove any potential formatting discrepancies
        # which don't affect the test of the content.
        expect(file_generator.preview_output.delete(' ')).to eq output.delete(' ')
      end

      it "creates a .editorconfig file in the correct location" do
        dot_editorconfig = file_generator.generate_config_file('test_output/.editorconfig')
        expect(dot_editorconfig.class).to eq File
        expect(File.exist? "test_output/.editorconfig").to eq true
      end
    end
    context "A collection of multiple EditorConfig objects is provided" do
      it "displays a preview of the file output" do
        configs = configs_sensible_defaults.push(EditorConfigGenerator::EditorConfig.new({file_type: '*.{js,py}', indent_size: 4}))
        file_generator = EditorConfigGenerator::FileGenerator.new(configs)
        output = "root = true
                [*]
                indent_style = space
                indent_size = 2
                end_of_line = lf
                charset = utf-8
                trim_trailing_whitespace = true
                insert_final_newline = true
                [*.{js,py}]
                indent_size = 4"

        expect(file_generator.preview_output.delete(' ')).to eq output.delete(' ')
      end
    end
  end
end

When I ran these tests the first few times, they worked seemingly fine. I then enabled the following recommended lines in my spec_helper.rb file

  config.profile_examples = 10
  config.order = :random
  kernel.srand config.seed

I ran the tests a few more times, all of them passing until - seemingly randomly - I obtained the following failure output with seed 31166:

  1) EditorConfigGenerator::FileGenerator A collection of one EditorConfig object is provided displays a preview of the file output
     Failure/Error: expect(file_generator.preview_output.delete(' ')).to eq output.delete(' ')

       expected: "root=true\n[*]\nindent_style=space\nindent_size=2\nend_of_line=lf\ncharset=utf-8\ntrim_trailing_whitespace=true\ninsert_final_newline=true\n"
            got: "root=true\n[*]\nindent_style=space\nindent_size=2\nend_of_line=lf\ncharset=utf-8\ntrim_trailing_whitespace=true\ninsert_final_newline=true\n[*.{js,py}]\nindent_size=4"

       (compared using ==)

       Diff:
       @@ -6,4 +6,6 @@
        charset=utf-8
        trim_trailing_whitespace=true
        insert_final_newline=true
       +[*.{js,py}]
       +indent_size=4

     # ./spec/file_generator_spec.rb:23:in `block (3 levels) in <module:EditorConfigGenerator>'

With other seeds I tried, it worked, but with seed 31166 it failed.

The Fix

The output made me feel like it was contextual, so therefore I tried to look for a bug in my implementation. I didn't find one, so I thought it may have been an issue with the way I defined the shared variable configs_sensible_defaults in the spec.

I decided to change the code to use before(:each) to assign an instance variable before each test (i.e. resetting the data properly) and it seemed to fix it.

Here's the fixed spec/file_generator_spec.rb

module EditorConfigGenerator
  RSpec.describe EditorConfigGenerator::FileGenerator do
    before(:each) do
      @configs_sensible_defaults =  [EditorConfigGenerator::EditorConfig.new({root: true, indent_style: 'space', indent_size: 2, end_of_line: 'lf', charset: 'utf-8', trim_trailing_whitespace: true, insert_final_newline: true})]
      @file_generator = EditorConfigGenerator::FileGenerator.new(@configs_sensible_defaults)
    end

    context "A collection of one EditorConfig object is provided" do
      it "displays a preview of the file output" do
        # Newlines are automatically inserted into ruby multi-line strings
        output = "root = true
                [*]
                indent_style = space
                indent_size = 2
                end_of_line = lf
                charset = utf-8
                trim_trailing_whitespace = true
                insert_final_newline = true
      "
        # String.delete(' ')  to remove any potential formatting discrepancies
        # which don't affect the test of the content.
        expect(@file_generator.preview_output.delete(' ')).to eq output.delete(' ')
      end

      it "creates a .editorconfig file in the correct location" do
        dot_editorconfig = @file_generator.generate_config_file('test_output/.editorconfig')
        expect(dot_editorconfig.class).to eq File
        expect(File.exist? "test_output/.editorconfig").to eq true
      end
    end
    context "A collection of multiple EditorConfig objects is provided" do
      it "displays a preview of the file output" do
        configs = @configs_sensible_defaults.clone.push(EditorConfigGenerator::EditorConfig.new({file_type: '*.{js,py}', indent_size: 4}))
        @file_generator = EditorConfigGenerator::FileGenerator.new(configs)
        output = "root = true
                [*]
                indent_style = space
                indent_size = 2
                end_of_line = lf
                charset = utf-8
                trim_trailing_whitespace = true
                insert_final_newline = true
                [*.{js,py}]
                indent_size = 4"

        expect(@file_generator.preview_output.delete(' ')).to eq output.delete(' ')
      end
    end
  end
end

Here is a diff output of both files (view on Github):

 RSpec.describe EditorConfigGenerator::FileGenerator do
    configs_sensible_defaults = [EditorConfigGenerator::EditorConfig.new({root: true, indent_style: 'space', indent_size: 2,
      end_of_line: 'lf', charset: 'utf-8',
    trim_trailing_whitespace: true, insert_final_newline: true})]
    file_generator = EditorConfigGenerator::FileGenerator.new(configs_sensible_defaults)
    before(:each) do
      @configs_sensible_defaults =  [EditorConfigGenerator::EditorConfig.new({root: true, indent_style: 'space', indent_size: 2, end_of_line: 'lf', charset: 'utf-8', trim_trailing_whitespace: true, insert_final_newline: true})]
      @file_generator = EditorConfigGenerator::FileGenerator.new(@configs_sensible_defaults)
    end

    context "A collection of one EditorConfig object is provided" do
      it "displays a preview of the file output" do
@ file_generator_spec.rb:22 @ module EditorConfigGenerator
      "
        # String.delete(' ')  to remove any potential formatting discrepancies
        # which don't affect the test of the content.
        expect(file_generator.preview_output.delete(' ')).to eq output.delete(' ')
        expect(@file_generator.preview_output.delete(' ')).to eq output.delete(' ')
      end

      it "creates a .editorconfig file in the correct location" do
        dot_editorconfig = file_generator.generate_config_file('test_output/.editorconfig')
        dot_editorconfig = @file_generator.generate_config_file('test_output/.editorconfig')
        expect(dot_editorconfig.class).to eq File
        expect(File.exist? "test_output/.editorconfig").to eq true
      end
    end
    context "A collection of multiple EditorConfig objects is provided" do

Finally, for context, here are the relevant file_generator.rb and editor_config.rb files

module EditorConfigGenerator
  class FileGenerator
    def initialize(configs)
      @configs = configs
    end

    def preview_output
      output = ""
      @configs.each do |config|
        if output.include? "root="
          output << config.to_s_without_root
          next
        end
        output << config.to_s
      end
      return output.rstrip if @configs.size > 1
      output
    end

    def generate_config_file(location='.editorconfig')
      File.delete(location) if File.exist? location
      file = File.new(location, "w")
      file.print(preview_output)
      file.close
      file
    end
  end
end

module EditorConfigGenerator
  class EditorConfig
    attr_reader :root, :indent_style, :indent_size,
    :end_of_line, :charset, :trim_trailing_whitespace,
    :insert_final_newline, :file_type

    def initialize(options)
      @root = nil
      @file_type = :*

      transform_options(options)
      set_options(options)
    end

    def set_options(options)
      @root = options[:root] unless options[:root].nil?
      @indent_style = options[:indent_style] unless options[:indent_style].nil?
      @indent_size = options[:indent_size] unless options[:indent_size].nil?
      @end_of_line = options[:end_of_line] unless options[:end_of_line].nil?
      @charset = options[:charset] unless options[:charset].nil?
      @trim_trailing_whitespace = options[:trim_trailing_whitespace] unless options[:trim_trailing_whitespace].nil?
      @insert_final_newline = options[:insert_final_newline] unless options[:insert_final_newline].nil?
      @file_type = options[:file_type] unless options[:file_type].nil?
    end

    def transform_options(options)
      options[:root] = true if options[:root] == 'y'
      options[:root] = false if options[:root] == 'n'
      options[:trim_trailing_whitespace] = true if options[:trim_trailing_whitespace] == 'y'
      options[:trim_trailing_whitespace] = false if options[:trim_trailing_whitespace] == 'n'
      options[:insert_final_newline] = true if options[:insert_final_newline] == 'y'
      options[:insert_final_newline] = false if options[:insert_final_newline] == 'n'
    end

    def to_s
      config_string = ""
      config_string << "root = #{@root.to_s}\n" unless @root.nil?
      config_string << "[#{@file_type}]\n"
      config_string << "indent_style = #{@indent_style}\n" unless @indent_style.nil?
      config_string << "indent_size = #{@indent_size}\n" unless @indent_size.nil?
      config_string << "end_of_line = #{@end_of_line}\n" unless @end_of_line.nil?
      config_string << "charset = #{@charset}\n" unless @charset.nil?
      config_string << "trim_trailing_whitespace = #{@trim_trailing_whitespace.to_s}\n" unless @trim_trailing_whitespace.nil?
      config_string << "insert_final_newline = #{@insert_final_newline.to_s}\n" unless @insert_final_newline.nil?
      config_string
    end

    def to_s_without_root
      lines = to_s.lines
      lines.delete_at 0
      lines.join
    end

    def to_s_without_line(line_identifier)
      lines = to_s.lines
      index = lines.index(line_identifier)
      lines.delete_at index
      lines.join
    end
  end
end

The Question

Could someone explain exactly why the change fixed the issue? Did the change fix the issue?

I believe it's to do with the collection in configs_sensible_defaults being mutated and not being reset correctly after each run, which would explain why a certain seed would trigger a failure (as maybe under that seed test 2 would run before 1).

Upvotes: 0

Views: 62

Answers (1)

Jim Stewart
Jim Stewart

Reputation: 17323

The reason it was working sometimes and not other times is because one of your tests (in the second context) reassigned file_generator. The test in the first context was using the file_generator from the shared outer scope. So long as that test ran first, it worked as expected. Then the second test in the second scope ran, reassigned file_generator, and also passed.

Since the tests ran in random order, whenever they ran in the order they are in the file, everything was passing. When the second test ran first, it overwrote file_generator, the second test still passed, but the first test ran with the overwritten value.

You can use a before block to configure things for each test, as you did, but you should only configure common, shared values in a before block. Anything that's going to be different for each context or each test should be initialized closer to the scope in which it is used.

In your examples above, I wouldn't use any before block, at least not in the outer scope. Both @config_sensible_defaults and @file_generator are different in each context, and should not be shared at all. If you want to group a bunch of tests together in a context with the same defaults and generator, you can put a before block inside the context block, to initialize things correctly for each context.

Upvotes: 1

Related Questions