From c1e75bca552218e0d5a1ab7228237ebc67ef34cd Mon Sep 17 00:00:00 2001
From: Malcolm Locke <mlocke@gitlab.com>
Date: Mon, 19 Feb 2024 14:48:23 +0000
Subject: [PATCH] Stub exist? in FileReadHelpers
It's a reasonable expectation that if `File.read` should work for
a stubbed filename then `File.exist?` should return true for the
same value.
---
.../testing_guide/best_practices.md | 19 ++++---
spec/support/helpers/file_read_helpers.rb | 54 ++++++++++---------
2 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md
index f5af911e802ca..d355b613a1c53 100644
--- a/doc/development/testing_guide/best_practices.md
+++ b/doc/development/testing_guide/best_practices.md
@@ -1194,26 +1194,29 @@ a single test triggers the rate limit, the `:disable_rate_limit` can be used ins
In the situations where you need to
[stub](https://rspec.info/features/3-12/rspec-mocks/basics/allowing-messages/)
-methods such as `File.read`, make sure to:
+the contents of a file use the `stub_file_read`, and
+`expect_file_read` helper methods which handle the stubbing for
+`File.read` correctly. These methods stub `File.read` for the given
+filename, and also stub `File.exist?` to return `true`.
-1. Stub `File.read` for only the file path you are interested in.
-1. Call the original implementation for other file paths.
+If you need to manually stub `File.read` for any reason be sure to:
+
+1. Stub and call the original implementation for other file paths.
+1. Then stub `File.read` for only the file path you are interested in.
Otherwise `File.read` calls from other parts of the codebase get
-stubbed incorrectly. You should use the `stub_file_read`, and
-`expect_file_read` helper methods which does the stubbing for
-`File.read` correctly.
+stubbed incorrectly.
```ruby
# bad, all Files will read and return nothing
allow(File).to receive(:read)
# good
-stub_file_read(my_filepath)
+stub_file_read(my_filepath, content: "fake file content")
# also OK
allow(File).to receive(:read).and_call_original
-allow(File).to receive(:read).with(my_filepath)
+allow(File).to receive(:read).with(my_filepath).and_return("fake file_content")
```
#### File system
diff --git a/spec/support/helpers/file_read_helpers.rb b/spec/support/helpers/file_read_helpers.rb
index c30a9e6466f92..9e5517415a409 100644
--- a/spec/support/helpers/file_read_helpers.rb
+++ b/spec/support/helpers/file_read_helpers.rb
@@ -1,24 +1,37 @@
# frozen_string_literal: true
module FileReadHelpers
- def stub_file_read(file, content: nil, error: nil)
- allow_original_file_read
+ def stub_file_read(file, content: nil, error: nil, exist: true)
+ allow_or_expect_file_read(
+ file, mode: :allow, content: content, error: error, exist: exist
+ )
+ end
- expectation = allow(File).to receive(:read).with(file)
+ def expect_file_read(file, content: nil, error: nil, exist: true)
+ allow_or_expect_file_read(
+ file, mode: :expect, content: content, error: error, exist: exist
+ )
+ end
- if error
- expectation.and_raise(error)
- elsif content
- expectation.and_return(content)
- else
- expectation
- end
+ def expect_file_not_to_read(file)
+ allow_original_file_calls
+
+ expect(File).not_to receive(:read).with(file)
end
- def expect_file_read(file, content: nil, error: nil)
- allow_original_file_read
+ private
+
+ def allow_or_expect_file_read(file, mode:, content:, error:, exist:)
+ allow_original_file_calls
+ allow(File).to receive(:exist?).with(file).and_return(exist)
- expectation = expect(File).to receive(:read).with(file)
+ expectation = if mode == :allow
+ allow(File).to receive(:read).with(file)
+ elsif mode == :expect
+ expect(File).to receive(:read).with(file)
+ else
+ raise ArgumentError, "expected :allow or :expect for `mode`, got `#{mode}`"
+ end
if error
expectation.and_raise(error)
@@ -29,20 +42,13 @@ def expect_file_read(file, content: nil, error: nil)
end
end
- def expect_file_not_to_read(file)
- allow_original_file_read
-
- expect(File).not_to receive(:read).with(file)
- end
-
- private
-
- def allow_original_file_read
+ def allow_original_file_calls
# Don't set this mock twice, otherwise subsequent calls will clobber
# previous mocks
- return if @allow_original_file_read
+ return if @allow_original_file_calls
- @allow_original_file_read = true
+ @allow_original_file_calls = true
allow(File).to receive(:read).and_call_original
+ allow(File).to receive(:exist?).and_call_original
end
end
--
GitLab