Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve RSpec/VerifiedDoubles todos #14400

Merged
merged 20 commits into from Jan 23, 2023
Merged

Conversation

dduugg
Copy link
Sponsor Member

@dduugg dduugg commented Jan 23, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This PR resolves the RSpec/VerifiedDoubles in Library/Homebrew/test/.rubocop_todo.yml

Relates to #14250 (apologies if this stepping on someone's toes, it's assigned but has been idle)

@dduugg dduugg force-pushed the rspec-cop-fix branch 2 times, most recently from 25e0cd0 to 1262dd9 Compare January 23, 2023 01:23
@@ -11,6 +11,8 @@ module Dependable
# misuse in future.
RESERVED_TAGS = [:build, :optional, :recommended, :run, :test, :linked].freeze

attr_reader :tags
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit tangential, but moving the attr here reduces duplication, allows us to remove the matching .rbi file, and write better tests.

@@ -3,10 +3,6 @@

describe Cask::Installer, :cask do
describe "install" do
let(:empty_depends_on_stub) {
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears to be unused (the last reference was removed in 7d5b8a5cea122c09f297dffa6b76187c413a9565)

clang_build_version: Version.create("600"),
)
end
let(:versions) { class_double(DevelopmentTools, clang_build_version: Version.create("600")) }
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the correct class_double, but it doesn't recognize llvm_build_version

let(:hash1) { double(hash_type: "sha256", to_s: "deadbeef") }
let(:hash2) { double(hash_type: "sha256", to_s: "deadcafe") }
let(:hash1) { instance_double(Checksum, to_s: "deadbeef") }
let(:hash2) { instance_double(Checksum, to_s: "deadcafe") }
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is the correct instance_double, but it doesn't recognize hash_type

@@ -246,11 +246,6 @@
end

describe "::create" do
it "accepts objects responding to #to_str" do
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this test because I think this premise is invalid. The sig for create require the input to be a String, and anything else will result in a TypeError, even if it responds to to_str:

sig { params(val: String).returns(Version) }

@MikeMcQuaid MikeMcQuaid merged commit f22f3ea into Homebrew:master Jan 23, 2023
@MikeMcQuaid
Copy link
Member

Wonderful, thanks so much for this @dduugg, this is great work! Hope to see more PRs from you here, all yours so far have been perfect and make me happy 😀

@dduugg dduugg deleted the rspec-cop-fix branch January 23, 2023 17:42
@dduugg dduugg mentioned this pull request Jan 23, 2023
7 tasks
@github-actions github-actions bot added the outdated PR was locked due to age label Feb 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants