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.

Sign up for GitHub

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

Jump to bottom

Fix pending specs #10

Merged
merged 4 commits into from May 16, 2014
Merged

Fix pending specs #10

merged 4 commits into from May 16, 2014

Conversation

myronmarston
Copy link
Member

Follow up to #9. Based on rspec/rspec-core#1525.

This is the last change I had planned to make before this being ready to release with the RC release. One thing I was concerned/confused about is that there are a bunch of places in the code here that specifically reference RSpec2 (or Rspec2). @JonRowe should we update those to rspec 3? I don't really understand how our autotest integration works, TBH.

@JonRowe
Copy link
Member

JonRowe commented May 13, 2014

So this LGTM bar the temporary branch, naturally.

@JonRowe
Copy link
Member

JonRowe commented May 13, 2014

Re: RSpec2 references, I'm pretty sure this means "> 2.0.x", we could update the references accordingly... but that might break 1.x... who are far more likely to be users of this...

@JonRowe
Copy link
Member

JonRowe commented May 13, 2014

Actually thinking about it... this was extracted from elsewhere, so that last statement is false. Maybe we can drop that stuff.

@myronmarston
Copy link
Member Author

Can we just change it to RSpec so it doesn't specify a version?

@myronmarston
Copy link
Member Author

@JonRowe I changed Rspec2 to RSpec and I also removed the .rspec detection conditional in order to address rspec/rspec-core#1051. Let me know what you think.

@JonRowe
Copy link
Member

JonRowe commented May 13, 2014

Hmm, so this looks fine from a 3.x point of view, but I do wonder what this will do when included in 2.99 though...

@myronmarston
Copy link
Member Author

Well, it doesn't specify an version in the discovery name anymore so why would it matter?

@myronmarston
Copy link
Member Author

I guess maybe we should try though

@JonRowe
Copy link
Member

JonRowe commented May 13, 2014

Mostly I think because it might conflict with the version built into 2.99

@myronmarston
Copy link
Member Author

So you were right...this didn't work quite right. I made a change to rspec-core to make this work:

rspec/rspec-core#1528. That plus a fix here (dbaf9ce) makes things work properly. I went through the 2.14 -> 2.99 -> 3.0 upgrade sequence and it works.

@JonRowe
Copy link
Member

JonRowe commented May 14, 2014

Ok, let's run with this then.

@myronmarston
Relax failed_results_re.
bd5c2d3 
rspec-core’s formatting has changed a bit. The
backtrace used to be formatted like so:

�[36m     # ./spec/autotest/failed_results_re_spec.rb:31:in `block (4 levels) in <top (required)>'�[0m
�[36m     # ./spec/autotest/failed_results_re_spec.rb:6:in `block in run_example'�[0m
�[36m     # ./spec/autotest/failed_results_re_spec.rb:18:in `run_group'�[0m
�[36m     # ./spec/autotest/failed_results_re_spec.rb:8:in `run_example'�[0m
�[36m     # ./spec/autotest/failed_results_re_spec.rb:31:in `block (3 levels) in <top (required)>'�[0m
�[36m     # /Users/myron/code/rspec-dev/repos/rspec-core/exe/rspec:4:in `<top (required)>'�[0m

…but is now formatted like so:

     �[36m# ./spec/autotest/failed_results_re_spec.rb:31:in `block (4 levels) in <top (required)>'�[0m
     �[36m# ./spec/autotest/failed_results_re_spec.rb:6:in `block in run_example'�[0m
     �[36m# ./spec/autotest/failed_results_re_spec.rb:18:in `run_group'�[0m
     �[36m# ./spec/autotest/failed_results_re_spec.rb:8:in `run_example'�[0m
     �[36m# ./spec/autotest/failed_results_re_spec.rb:31:in `block (3 levels) in <top (required)>'�[0m
     �[36m# /Users/myron/code/rspec-dev/repos/rspec-core/exe/rspec:4:in `<top (required)>'�[0m

This change makes the regex tolerant of the ANSI color code
being anywhere between the start of the line and the `#`.
@myronmarston
Rspec2 -> RSpec
ca610d7 
There’s no good reason to include the rspec version number.
@myronmarston
Make this gem always work, not just when .rspec is defined.
c9f2234 
This addresses rspec/rspec-core#1051
@myronmarston
Autotest::RSpec -> Autotest::Rspec.
a031246 
Autotest’s auto discovery mechanism only works properly
when we name the constant `Rspec` since that’s how it
maps `rspec`.
@myronmarston
Copy link
Member Author

OK, I removed the temporary branch usage now that the rspec-core PR got merged. I plan to merge this when green.

JonRowe added a commit that referenced this pull request May 16, 2014
@JonRowe
Merge pull request #10 from rspec/fix-pending-specs
d2a9a22 
Fix pending specs
@JonRowe JonRowe merged commit d2a9a22 into master May 16, 2014
@JonRowe
Copy link
Member

JonRowe commented May 16, 2014

Merging as green.

@JonRowe JonRowe deleted the fix-pending-specs branch May 16, 2014 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@myronmarston @JonRowe