🌈 Fix readline specs#717
Merged
Merged
Conversation
An error was introduced in commit 7061b06 where readline specs fail because `require "readline"` is be called after the spec has established a mock `::Readline`. Prior to that change `require "readline"` was always only called once. This patch: * memoizes the `require` to ensure that it only happens once * leaves the `Object.const_defined?` check in place to keep tests simple * eagerly calls `available?` in the readline spec to match the original author's intent by requiring `readline` before the specs start.
rafaelfranca
reviewed
Mar 5, 2020
| class Readline < Basic | ||
| def self.available? | ||
| begin | ||
| require "readline" |
Member
There was a problem hiding this comment.
require by default only happens once. Why do we need this memoization?
Contributor
Author
There was a problem hiding this comment.
I had myself convinced that subsequent calls after building the mock would raise a TypeError here, but I was incorrect. Will update this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An error was introduced in commit 7061b06
where readline specs fail because
require "readline"is called after thespec has established a mock
::Readline. Prior to that changerequire "readline"was always only called once.This patch:
requireto ensure that it only happens onceObject.const_defined?check in place to keep tests simpleavailable?in the readline spec to match the originalauthor's intent by requiring
readlinebefore the specs start.This error wasn't caught by CI due to the bundler cache. You can see the failure caused by the earlier change by removing the Bundler cache from Travis