Enabling RSpec/VoidExpect

[robaiken]

https://docs.rubocop.org/rubocop-rspec/cops_rspec.html#rspecvoidexpect

[jurre]

This inverts the expectation doesn't it?

[jurre]

Very confused why this doesn't break any of the tests 😬

[robaiken]

Yea, @jurre , it shouldn't work, but I don't want to delay this epic for a broken test. I can either skip the test or leave it as it is, but I will raise an issue.

[jurre]

I'm just surprised that it was passing before. Is there a subtle difference between File.empty? and File.read.empty? maybe? Perhaps when the file does not exist?

File.empty? (alias of File.zero?):

Returns true if the named file exists and has a zero size.

Whereas File.read would raise if the file doesn't exist. Which, wouldn't really explain the test here passing. I also checked with something like a space, but behaves the same there, as expected 🤔

[landongrindheim]

I assume .npmrc has a zero size. File.read won't blow up if the file exists (which it does, as shown in the expectation a few lines up), but is empty.

 dependabot@codespaces-a9309d:/app$ touch .npmrc
dependabot@codespaces-a9309d:/app$ irb
irb(main):001> File.read(".npmrc").empty?
=> true

File.read returns a String, and String#empty? will have different implications.

[robaiken]

Is this expected behaviour? Should I update the test to reflect this? It seems like a bug and we should log it as an issue

[landongrindheim]

Is this expected behaviour?

There are enough references in this thread that I'm not 100% sure which behavior you're referring to 🙈

My hope is that the (perceived) intent of the test will remain. The expectation could be updated to maintain the current behavior with something like 👇

Suggested change

	expect(File.read(".npmrc")).not_to be_empty
	expect(File.empty?(".npmrc")).to be_truthy

[robaiken]

@landongrindheim it looks like File.empty?(".npmrc") returns false.

There are enough references in this thread that I'm not 100% sure which behavior you're referring to 🙈

Lol, sorry, you are right. The behavior I am referring to is whether .npmrc should exist in this scenario. It seems like the original test was not set up correctly as it did not have an assertion, so it passes every time. If that's the case, then it's weird that RSpec doesn't log that the test has no assertions.

[landongrindheim]

@robaiken 👍 In that case, I think it makes sense to make the test honest and ensure that the assertions are inline with the intent of the tests. Good find!