Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Ignore working directory when excluding system patterns.#1616

Merged
JonRowe merged 3 commits into
3-0-maintenancefrom
fix_exclusion_priority_for_working_directory
Jul 16, 2014
Merged

Ignore working directory when excluding system patterns.#1616
JonRowe merged 3 commits into
3-0-maintenancefrom
fix_exclusion_priority_for_working_directory

Conversation

@JonRowe
Copy link
Copy Markdown
Member

@JonRowe JonRowe commented Jun 24, 2014

Fixes #1604 by removing working directory from excluded patterns.

@JonRowe
Copy link
Copy Markdown
Member Author

JonRowe commented Jun 24, 2014

FYI I'm about to get on a flight to singapore for RDRC so won't reply to feedback till later.

Comment thread lib/rspec/core/backtrace_formatter.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parens, please...

JonRowe and others added 2 commits June 25, 2014 09:18
- The original spec had a double-slashed path like
  '/Code/gems//arbitrary'.
- Stubbing `Dir.getwd` forced the implementation to
  use that, but we'd like to reuse `RSpec::Metadata.relative_path`
  which does not use `Dir.getwd` internally.
@myronmarston
Copy link
Copy Markdown
Member

@JonRowe -- I've revisited this and applied some further improvements to what you had before (see the commit messages for rationale). Thoughts?

- doesnt_match_inclusion_pattern_unless_system_exclusion?
  has a double negative and was being used in a compound
  conditional, which made my head explode. I couldn't
  understand it at all.
- Reuse `Metadata.relative_path` rather than having our own
  logic to do that.
- It'll perform better to get the relative path once rather
  than doing it once per pattern in the `patterns.any?` block.
- Add spec for case that is covered by a cucumber scenario but
  not by an existing spec (an earlier version of this commit
  broke that cucumber scenario but no specs failed, revealing
  the missing spec coverage).
@JonRowe
Copy link
Copy Markdown
Member Author

JonRowe commented Jul 16, 2014

Seems legit.

JonRowe added a commit that referenced this pull request Jul 16, 2014
…g_directory

Ignore working directory when excluding system patterns.
@JonRowe JonRowe merged commit b7548b3 into 3-0-maintenance Jul 16, 2014
@JonRowe JonRowe deleted the fix_exclusion_priority_for_working_directory branch July 16, 2014 03:34
JonRowe added a commit that referenced this pull request Jul 16, 2014
JonRowe added a commit that referenced this pull request Jul 16, 2014
…g_directory

Ignore working directory when excluding system patterns.
JonRowe added a commit that referenced this pull request Jul 16, 2014
@JonRowe
Copy link
Copy Markdown
Member Author

JonRowe commented Jul 16, 2014

Picked this to master too.

myronmarston added a commit that referenced this pull request 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 pull request 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 pull request 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 pull request Oct 30, 2020
…orking_directory

Ignore working directory when excluding system patterns.
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020
MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request 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 pull request Oct 6, 2021
…ion_priority_for_working_directory

Ignore working directory when excluding system patterns.

---
This commit was imported from rspec/rspec-core@b7548b3.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 6, 2021
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request 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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants