This repository was archived by the owner on Nov 30, 2024. It is now read-only.
Fix for #1280 #1289
Merged
Fix for #1280 #1289
Conversation
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
|
I think I'd prefer they be lazily compiled, to give people the most chance to set things like pattern, but it will lead to some confusing behaviour (e.g. if you check theres only one file before setting pattern) |
It's easiest to leverage the same "order then set configs" logic of `process_options_into` rather than treating libs and requires as special.
The old way was odd. It created an ordering dependency (you had to call `parse_options` after instantiating the object but before doing anything else) and calling it a second time had the side effect of re-parsing the args, which wasn't needed or intended. It's far easier/better to make `options` do the parsing and memoize the result.
…a `-r`. This changes it to be lazily rather than eagerly computed. This is important to allow `pattern` to be set from a `spec_helper` file loaded via `-r`.
|
I went with the lazy solution. Can you review this, @JonRowe? |
| And I create a spec file with the following content: | ||
| """ | ||
| And I create "spec/addition_spec.rb" with the following content: | ||
| """ruby |
There was a problem hiding this comment.
You mean the """ruby bit? Yes, that's for relish -- it tells it what kind of text it is, so it can apply syntax highlighting.
|
Looks good to me, and surprisingly simple... |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is the start of a fix for #1280. So far I've only gotten as far as a test exposing the issue.
The "easy" fix for this is to reorder things so that the
files_to_runare resolved beforerequiresare processed. However, that would break the fix @JonRowe put in place in 25d6b3a, which allowspatternto be set in anRSpec.configureblock if you use a CLI option to require your spec helper.patterncan affectfiles_to_runso the change he made makes sense. I'm not sure what the best fix is. A few ideas:files_to_run, rather than being assigned at a particular point in time, is lazily computed on first access. If the user setspatternbefore accessingfiles_to_runit would take the pattern into account but otherwise it wouldn't. This might require a large amount of refactoring.config.running_one_spec_file?that is set to true if and only if a single file name has been passed at the command line. I think this can be computed apart from thepatternso it could happen earlier.@JonRowe, what are your thoughts, since you've worked in this area before?
I'm interested in everyone's feedback, though.
/cc @xaviershay @alindeman @soulcutter @samphippen