Extend the tests in regexp/inspect_spec with over escaping combined with character classes
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 622
- Forks
- 402
- Avg merge
- 14h 51m
- Merged PRs (30d)
- 6
Description
The specs have one test for escaped backslashes in Regexp#inspect:
it "does not over escape" do
Regexp.new('\\\/').inspect.should == "/\\\\\\//"
end
The obvious implementation to make this spec pass is to simply escape every slash with another slash. This works in this case, but that causes an error in language/case_spec for something that is not related to the case statement:
it "tests with a regexp interpolated within another regexp" do
digits_regexp = /\d+/
case "foo43"
when /oo(#{digits_regexp})/
This results in a regexp /oo(?-mix:\\d+)/, which now expects a slash followed by one or more d characters. This does not match the input, but does not indicate the escaping as the issue.
I tested my implementation with the these specs added, but this was more or less brute forcing every possibility. Also, because Regexp.new takes a String which has an additional layer of escaping, I peferred the // notation.
Regexp.new('\d+').inspect.should == "/\\d+/"
/\d+/.inspect.should == "/\\d+/"
Regexp.new('\\d+').inspect.should == "/\\d+/"
/\\d+/.inspect.should == "/\\\\d+/"
Regexp.new('\\\d+').inspect.should == "/\\\\d+/"
/\\\d+/.inspect.should == "/\\\\\\d+/"
Regexp.new('\\\\d+').inspect.should == "/\\\\d+/"
/\\\\d+/.inspect.should == "/\\\\\\\\d+/"
I think it would be better to at least include the following cases:
/\d+/ # Output should have 1 not escaped slash, since this should not be escaped
/\\d+/ # Output should have 1 escaped slash (so 2 slashes), this should be escaped
/\\\d+/ # Output should have 1 escaped and 1 not escaped slash (so 3 slashes)
Furthermore, I think it would be better to add an explicit test for the regexp embedded in regexp scenario like there is in the case spec now, just to make the test a bit more explicit and decouple it from the case statement.
I'm not that familiar with this project and how things are structured, can I just add a few tests to the inspect spec of regexp, and make a new spec file for the embedding tests?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with regexp/inspect_spec and the existing escaped-backslash example, then review language/case_spec for the interpolated-regexp behavior. Add coverage for the listed backslash and character-class cases and an explicit embedded-regexp example; the relevant specs should pass without changing the case-statement behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100