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

RSpec excludes backtrace from working directory called "gems" #1604

Closed
radar opened this issue Jun 13, 2014 · 11 comments
Closed

RSpec excludes backtrace from working directory called "gems" #1604

radar opened this issue Jun 13, 2014 · 11 comments
Assignees

Comments

@radar
Copy link
Contributor

radar commented Jun 13, 2014

I have all my gems at a directory called ~/Projects/gems. I've done this for a couple of years now.

When I started a new project yesterday using RSpec 3 (https://github.com/radar/spree_poro), I noticed that it was showing the entire backtrace, rather than excluding all the lines from RSpec and only showing the lines from my code.

I think this is due to backtrace_exclusion_patterns checking for "gems" in the path, as seen here.

I understand the reasons behind that line being there; it's just a way to blanket exclude all the code from the Ruby install's gem directory. I think this is fixable in one of two ways: 1) I rename my gems directory to something else less generic or 2) RSpec checks something equivalent to gem env to find the "GEM PATHS" and excludes those directories.

@JonRowe
Copy link
Member

JonRowe commented Jun 13, 2014

I'd be ok with us relying on ENV['GEM_PATH'] and not filtering them out if it's not set for some reason...

@myronmarston
Copy link
Member

As of #843 (released in RSpec 2.14), there's also backtrace_inclusion_patterns which specifies lines to always include in the backtrace, even if the line matches an exclusion pattern. The default value is just the current working directory, to specifically handle cases like yours:

@inclusion_patterns = [Regexp.new(Dir.getwd)]

So there's something else going on here that I don't understand yet. We're also discussing removing the gems part of the backtrace filtering altogether (#1554) based on the discussion in #1536...so if we do that, this may be a moot point.

@JonRowe
Copy link
Member

JonRowe commented Jun 13, 2014

Could this be related to #1602?

@myronmarston
Copy link
Member

@radar -- to repro this, should we just clone your spree_poro repo into ~/Projects/gems/spree_poro and run the specs? Is there a particular branch or SHA we should check out?

@radar
Copy link
Contributor Author

radar commented Jun 22, 2014

Yup, that's all! No particular SHA, master branch works well.

On Sun, Jun 22, 2014 at 3:47 PM, Myron Marston notifications@github.com
wrote:

@radar -- to repro this, should we just clone your spree_poro repo into ~/Projects/gems/spree_poro and run the specs? Is there a particular branch or SHA we should check out?

Reply to this email directly or view it on GitHub:
#1604 (comment)

@myronmarston
Copy link
Member

Thanks @radar. I can repro this.

@JonRowe -- I've tracked this regression down to your changes in #1329. When I checkout the version of backtrace_formatter.rb from prior to that PR (and also remove this line that references a constant no longer defined), the backtrace formatter works properly (sans the issue that #1329 was solving). I never really dug into your changes in #1329 to understand them...can you look into this, please?

@JonRowe
Copy link
Member

JonRowe commented Jun 23, 2014

Sure thing

@JonRowe
Copy link
Member

JonRowe commented Jun 24, 2014

Yep so found the reason for this, we made system exclusions take priority over inclusions and so excludes over the inclusion of the working directory, a simple fix for this I think is to exclude the working directory from the lines matched against exclusion filters

@radar
Copy link
Contributor Author

radar commented Jun 24, 2014

Thanks for taking a look at this @myronmarston and @JonRowe :)

@myronmarston
Copy link
Member

This should be fixed now that #1616 has been merged. @radar, want to give that a try? You can point your gemfile at our 3-0-maintenance branch to try it out.

@radar
Copy link
Contributor Author

radar commented Jul 16, 2014

Yup, this works now. Thank you @myronmarston and @JonRowe :)

myronmarston added a commit that referenced this issue Sep 17, 2014
This also greatly simplifies the backtrace formatter;
the system_exclusion_patterns approach made it more
complicated, as did always having an inclusion_pattern.

We only need the current directory as an inclusion_pattern
when it matches one of the built-in exclusion patterns.

Note that this was the approach that we originally had in #843;
it got lost in the refactoring in #1062 (af0b271).
While it seemed like a simpler approach to always put the current dir
in the inclusion_patterns, in practice, this caused a sequence
of whack-a-mole regressions:

- #1328 (fixed by #1329)
- #1604 (fixed by #1616)
- #1705 (fixed by this commit)

I'm hopeful that returning to only including the current dir
in `inclusion_patterns` if it matches a built-in exclusion pattern
will solve these issues once and for all.
myronmarston added a commit that referenced this issue Sep 17, 2014
This also greatly simplifies the backtrace formatter;
the system_exclusion_patterns approach made it more
complicated, as did always having an inclusion_pattern.

We only need the current directory as an inclusion_pattern
when it matches one of the built-in exclusion patterns.

Note that this was the approach that we originally had in #843;
it got lost in the refactoring in #1062 (af0b271).
While it seemed like a simpler approach to always put the current dir
in the inclusion_patterns, in practice, this caused a sequence
of whack-a-mole regressions:

- #1328 (fixed by #1329)
- #1604 (fixed by #1616)
- #1705 (fixed by this commit)

I'm hopeful that returning to only including the current dir
in `inclusion_patterns` if it matches a built-in exclusion pattern
will solve these issues once and for all.
myronmarston added a commit that referenced this issue Sep 18, 2014
This also greatly simplifies the backtrace formatter;
the system_exclusion_patterns approach made it more
complicated, as did always having an inclusion_pattern.

We only need the current directory as an inclusion_pattern
when it matches one of the built-in exclusion patterns.

Note that this was the approach that we originally had in #843;
it got lost in the refactoring in #1062 (af0b271).
While it seemed like a simpler approach to always put the current dir
in the inclusion_patterns, in practice, this caused a sequence
of whack-a-mole regressions:

- #1328 (fixed by #1329)
- #1604 (fixed by #1616)
- #1705 (fixed by this commit)

I'm hopeful that returning to only including the current dir
in `inclusion_patterns` if it matches a built-in exclusion pattern
will solve these issues once and for all.
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this issue Oct 30, 2020
This also greatly simplifies the backtrace formatter;
the system_exclusion_patterns approach made it more
complicated, as did always having an inclusion_pattern.

We only need the current directory as an inclusion_pattern
when it matches one of the built-in exclusion patterns.

Note that this was the approach that we originally had in rspec#843;
it got lost in the refactoring in rspec#1062 (af0b271).
While it seemed like a simpler approach to always put the current dir
in the inclusion_patterns, in practice, this caused a sequence
of whack-a-mole regressions:

- rspec#1328 (fixed by rspec#1329)
- rspec#1604 (fixed by rspec#1616)
- rspec#1705 (fixed by this commit)

I'm hopeful that returning to only including the current dir
in `inclusion_patterns` if it matches a built-in exclusion pattern
will solve these issues once and for all.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this issue Oct 6, 2021
This also greatly simplifies the backtrace formatter;
the system_exclusion_patterns approach made it more
complicated, as did always having an inclusion_pattern.

We only need the current directory as an inclusion_pattern
when it matches one of the built-in exclusion patterns.

Note that this was the approach that we originally had in rspec/rspec-core#843;
it got lost in the refactoring in rspec/rspec-core#1062 (af0b271c92ad2a3336eb70be7bf2c0de4325a568).
While it seemed like a simpler approach to always put the current dir
in the inclusion_patterns, in practice, this caused a sequence
of whack-a-mole regressions:

- rspec/rspec-core#1328 (fixed by rspec/rspec-core#1329)
- rspec/rspec-core#1604 (fixed by rspec/rspec-core#1616)
- rspec/rspec-core#1705 (fixed by this commit)

I'm hopeful that returning to only including the current dir
in `inclusion_patterns` if it matches a built-in exclusion pattern
will solve these issues once and for all.

---
This commit was imported from rspec/rspec-core@9f53daf.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants