This repository was archived by the owner on Nov 30, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 101
WIP: Update shared context to minimize loaded stdlibs. #179
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
a2d2656
Update shared context to minimize loaded stdlibs.
myronmarston aefe3aa
Add whitespace spec.
myronmarston 02fd168
Add support for skipping some spec files (used by rspec-core).
myronmarston ae426e8
Add missing rbconfig require.
myronmarston 4e1f0a6
Mark these specs as failing on AppVeyor.
myronmarston File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
require 'rbconfig' | ||
|
||
module RSpec | ||
module Support | ||
# @api private | ||
|
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
require 'rspec/support/spec/shell_out' | ||
|
||
RSpec.shared_examples_for "library wide checks" do |lib, options| | ||
consider_a_test_env_file = options.fetch(:consider_a_test_env_file, /MATCHES NOTHING/) | ||
allowed_loaded_feature_regexps = options.fetch(:allowed_loaded_feature_regexps, []) | ||
preamble_for_lib = options[:preamble_for_lib] | ||
preamble_for_spec = "require 'rspec/core'; require 'spec_helper'" | ||
skip_spec_files = options.fetch(:skip_spec_files, /MATCHES NOTHING/) | ||
|
||
include RSpec::Support::ShellOut | ||
|
||
define_method :files_to_require_for do |sub_dir| | ||
slash = File::SEPARATOR | ||
lib_path_re = /#{slash + lib}[^#{slash}]*#{slash}lib/ | ||
load_path = $LOAD_PATH.grep(lib_path_re).first | ||
directory = load_path.sub(/lib$/, sub_dir) | ||
files = Dir["#{directory}/**/*.rb"] | ||
extract_regex = /#{Regexp.escape(directory) + File::SEPARATOR}(.+)\.rb$/ | ||
|
||
# We sort to ensure the files are loaded in a consistent order, regardless | ||
# of OS. Otherwise, it could load in a different order on Travis than | ||
# locally, and potentially trigger a "circular require considered harmful" | ||
# warning or similar. | ||
files.sort.map { |file| file[extract_regex, 1] } | ||
end | ||
|
||
def command_from(code_lines) | ||
code_lines.join("\n") | ||
end | ||
|
||
def load_all_files(files, preamble, postamble=nil) | ||
requires = files.map { |f| "require '#{f}'" } | ||
command = command_from(Array(preamble) + requires + Array(postamble)) | ||
|
||
stdout, stderr, status = with_env 'NO_COVERAGE' => '1' do | ||
options = %w[ -w ] | ||
options << "--disable=gem" if RUBY_VERSION.to_f >= 1.9 && RSpec::Support::Ruby.mri? | ||
run_ruby_with_current_load_path(command, *options) | ||
end | ||
|
||
# Ignore bundler warning. | ||
stderr = stderr.split("\n").reject { |l| l =~ %r{bundler/source/rubygems} }.join("\n") | ||
[stdout, stderr, status.exitstatus] | ||
end | ||
|
||
define_method :load_all_lib_files do | ||
files = all_lib_files - lib_test_env_files | ||
preamble = ['orig_loaded_features = $".dup', preamble_for_lib] | ||
postamble = [ | ||
'loaded_features = ($" - orig_loaded_features).join("\n")', | ||
"File.open('#{loaded_features_outfile}', 'w') { |f| f.write(loaded_features) }" | ||
] | ||
|
||
load_all_files(files, preamble, postamble) | ||
end | ||
|
||
define_method :load_all_spec_files do | ||
files = files_to_require_for("spec") + lib_test_env_files | ||
files = files.reject { |f| f =~ skip_spec_files } | ||
load_all_files(files, preamble_for_spec) | ||
end | ||
|
||
attr_reader :loaded_features_outfile, :all_lib_files, :lib_test_env_files, | ||
:lib_file_results, :spec_file_results | ||
|
||
before(:context) do | ||
@loaded_features_outfile = if ENV['CI'] | ||
# On AppVeyor we get exit status 5 ("Access is Denied", | ||
# from what I've read) when trying to write to a tempfile. | ||
# | ||
# On Travis, we occasionally get Errno::ENOENT (No such file | ||
# or directory) when reading from a tempfile. | ||
# | ||
# In both cases if we leave a file behind in the current dir, | ||
# it's not a big deal so we put it in the current dir. | ||
File.join(".", "loaded_features.txt") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should just be |
||
else | ||
# Locally it's nice not to pollute the current working directory | ||
# so we use a tempfile instead. | ||
require 'tempfile' | ||
Tempfile.new("loaded_features.txt").path | ||
end | ||
|
||
@all_lib_files = files_to_require_for("lib") | ||
@lib_test_env_files = all_lib_files.grep(consider_a_test_env_file) | ||
|
||
@lib_file_results, @spec_file_results = [ | ||
# Load them in parallel so it's faster... | ||
Thread.new { load_all_lib_files }, | ||
Thread.new { load_all_spec_files } | ||
].map(&:join).map(&:value) | ||
end | ||
|
||
def have_successful_no_warnings_output | ||
eq ["", "", 0] | ||
end | ||
|
||
it "issues no warnings when loaded", :slow, :failing_on_appveyor do | ||
expect(lib_file_results).to have_successful_no_warnings_output | ||
end | ||
|
||
it "issues no warnings when the spec files are loaded", :slow do | ||
expect(spec_file_results).to have_successful_no_warnings_output | ||
end | ||
|
||
it 'only loads a known set of stdlibs so gem authors are forced ' \ | ||
'to load libs they use to have passing specs', :slow, :failing_on_appveyor do | ||
loaded_features = File.read(loaded_features_outfile).split("\n") | ||
if RUBY_VERSION == '1.8.7' | ||
# On 1.8.7, $" returns the relative require path if that was used | ||
# to require the file. LIB_REGEX will not match the relative version | ||
# since it has a `/lib` prefix. Here we deal with this by expanding | ||
# relative files relative to the $LOAD_PATH dir (lib). | ||
Dir.chdir("lib") { loaded_features.map! { |f| File.expand_path(f) } } | ||
end | ||
|
||
loaded_features.reject! { |feature| RSpec::CallerFilter::LIB_REGEX =~ feature } | ||
loaded_features.reject! { |feature| allowed_loaded_feature_regexps.any? { |r| r =~ feature } } | ||
|
||
expect(loaded_features).to eq([]) | ||
end | ||
|
||
# This malformed whitespace detection logic has been borrowed from bundler: | ||
# https://github.com/bundler/bundler/blob/v1.8.0/spec/quality_spec.rb | ||
def check_for_tab_characters(filename) | ||
failing_lines = [] | ||
File.readlines(filename).each_with_index do |line, number| | ||
failing_lines << number + 1 if line =~ /\t/ | ||
end | ||
|
||
return if failing_lines.empty? | ||
"#{filename} has tab characters on lines #{failing_lines.join(', ')}" | ||
end | ||
|
||
def check_for_extra_spaces(filename) | ||
failing_lines = [] | ||
File.readlines(filename).each_with_index do |line, number| | ||
next if line =~ /^\s+#.*\s+\n$/ | ||
failing_lines << number + 1 if line =~ /\s+\n$/ | ||
end | ||
|
||
return if failing_lines.empty? | ||
"#{filename} has spaces on the EOL on lines #{failing_lines.join(', ')}" | ||
end | ||
|
||
RSpec::Matchers.define :be_well_formed do | ||
match do |actual| | ||
actual.empty? | ||
end | ||
|
||
failure_message do |actual| | ||
actual.join("\n") | ||
end | ||
end | ||
|
||
it "has no malformed whitespace" do | ||
error_messages = [] | ||
`git ls-files -z`.split("\x0").each do |filename| | ||
error_messages << check_for_tab_characters(filename) | ||
error_messages << check_for_extra_spaces(filename) | ||
end | ||
expect(error_messages.compact).to be_well_formed | ||
end | ||
end |
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The irony of adding an additional piece of standard library while detecting usage of the standard library amuses me ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I noticed that rspec-core loads rbconfig:
https://github.com/rspec/rspec-core/blob/v3.2.0/lib/rspec/core.rb#L5
...but it's not used in rspec-core anywhere. Originally it was used for the OS detection that got moved here but the require never got moved. It should be here since it is used here. I will be removing it from rspec-core soon.