2

Maybe that's simple, but it confuses me. I have such a code now:

describe VarnishLogExaminer::Entry do
  let(:entry) do
    file_string = File.read('./spec/assets/varnish_example.xml')
    first_entry_string = file_string.split("\n").first
    described_class.new(first_entry_string)
  end
  describe 'initialize' do
    context 'when a proper Varnish log line is given' do
      it 'saves remote host properly' do
        expect(entry.remote_host_ip).to eql '85.164.152.30'
      end
      # ...
    end
  end
  describe 'to_h' do
    it 'returns hash with all entry properties' do
      expect(entry.to_h).to eql(
        # ...
      )
    end
  end
  # ... other methods using the entry instance
end

It seems ugly, because the initialization of the entry object happens in the let block, not in the context. The alternative could be initializing it once again in the context or making a method running in the context and before each of other methods, but in my opinion both of the options violate the DRY rule. What solution would be the best from the point of view of code readability and flexibility?

Karol Selak
  • 221
  • 2
  • 8
  • 2
    Does this answer your question? [What should I consider when the DRY and KISS principles are incompatible?](https://softwareengineering.stackexchange.com/questions/400183/what-should-i-consider-when-the-dry-and-kiss-principles-are-incompatible) – gnat Jun 05 '20 at 13:24

2 Answers2

1

I wouldn't apply DRY rules same way in the tests as we usually apply them in the production code. As you mention context in tests is very important for other when they read those tests.

Your sample in the question doesn't seem ugly(maybe add empty line between describes and let blocks :)).

As far as let is visible when you reading last test in the bottom, this looks ok, because reader can see how entry is initialised.

When user need to scroll, then I would wrap it with the method and call this method in every test case. Notice calling method wouldn't violate dry, because you not duplicating but delegating. You can add parameters to the method, then every test case can pass own values.

Actually, when you start writing tests, you can duplicate everything in every test case just to make it work. When you notice some pattern in what and how test case need to be prepared you can extract that logic into lets or methods with well descriptive names.

Fabio
  • 3,086
  • 1
  • 17
  • 25
  • 1
    [This question on Stack Overflow](https://stackoverflow.com/q/6453235/572) is highly relevant as well. But I can't add much else other than that link to this answer. – Thomas Owens Jun 05 '20 at 23:43
0

I don't know Ruby, but

"Making a method running in the context"

does not sound to me like violating DRY.

I guess you are concerned because one would have to call that method two times, in two places, and maybe because the code is executed two times? But that is not something which counts as a DRY violation.

DRY is about having core logic of a program in only one place, so when a requirement changes, for each concern you ideally have to change only one place in the code, not several ones. You don't want to have the file name and the entry initialization logic duplicated in two places in your code, not even in your tests, that would make it hard to maintain later.

By putting the code into a method, you achieve this goal, even if that method will be called in two places. If you later get new requirements like a changed folder structure for your XML files, or an additional initialization step for the entry object, only this method has to be changed, not several places in the code.

To keep things readable, however, the key point is to give the method a proper name. Avoid braindead names like InitEntry when the entry object is initialized with some specific data. Use a name like, for example, InitEntryWithVarnishExample (or whatever more descriptive name you can come up with).

Doc Brown
  • 199,015
  • 33
  • 367
  • 565