Fixed some rubocop warnings in the specs

[vheuken]

Started with 357 Rubocop offenses. Now we're down to 162.

I want to make sure this passes Hound-CI. Still need to do yotuube_api_spec, vkontakte_spec, and daily_motion_spec. If it passes Hound-CI, I'll merge it in and do the rest in another PR.

[thibaudgg]

I would not write it like that with a variable, but rather:

expect(provider.embed_code(iframe_attributes: { foo: 'bar' }))
  .to eq '<iframe src="//foo.com" frameborder="0" foo="bar"></iframe>'

same for all rewritten specs like that.

[vheuken]

Fair. I wasn't totally sold on it, either. Just seemed like the best way to do it at the time. I'll go back and clean those up this weekend. Should be pretty simple since I named them all expected

[thibaudgg]

👍🏻

[vheuken]

Bit of an issue: doing what you suggested gives:

C: Place the . on the previous line, together with the method call receiver.
        .to raise_error(NotImplementedError, error_msg)
        ^

Formatting it like so:

      expect { VideoInfo::Provider.new(url) }.to
        raise_error(NotImplementedError, error_msg)

Gives C: Inconsistent indentation detected.

The only way to format it in a way Rubocop likes is:

      expect { VideoInfo::Provider.new(url) }.to
      raise_error(NotImplementedError, error_msg)

....which I have a problem with since it is unclear that it is a continuation of the previous line.

Also, with the way I did it, we end up saving code in some places, like here:

    it 'should apply width and height attributes properly' do
      dimensions = { width: 800, height: 600 }
      expected = expect(subject.embed_code(iframe_attributes: dimensions))
      expected.to match(/width="800"/)
      expected.to match(/height="600"/)
    end

Whereas before, there were two expect function calls on the exact same data.

Obviously, we could go modify the rubocop config, but personally I'd rather not mess with that too much since I think what we're using (which is the Hound-CI default) is a solid standard.

Unless you have any other ideas, I'd propose on keeping it the way it is.

[vheuken]

If you have time, feel free to experiment with it and see if you can come up with a nicer solution. For now, though, I think I'm going to spend my time on the other tasks on the docket.

[thibaudgg]

In my work we use the following config:

Style/DotPosition:
  Description: Checks the position of the dot in multi-line method calls.
  StyleGuide: https://github.com/bbatsov/ruby-style-guide#consistent-multi-line-chains
  Enabled: true
  EnforcedStyle: leading
  SupportedStyles:
  - leading
  - trailing

I think it worths updating our config as well.

Regarding the 2nd point, I prefer:

    it 'should apply width and height attributes properly' do
      dimensions = { width: 800, height: 600 }
      embed_code = subject.embed_code(iframe_attributes: dimensions)
      expect(embed_code).to match(/width="800"/)
      expect(embed_code).to match(/height="600"/)
    end

[vheuken]

Sounds good. I'll modify the config accordingly.