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

test: Remove pattern with regex #48

Closed
gabssnake opened this issue Jan 12, 2017 · 4 comments
Closed

test: Remove pattern with regex #48

gabssnake opened this issue Jan 12, 2017 · 4 comments

Comments

@gabssnake
Copy link
Collaborator

Hi there, hope you are doing good.
Is there a way to remove a pattern that contains only a regex, as the one in the readme?

bloomrun.add({say: /.*/}, 'Matched with a regexp!')

Tried the following test:

// Test passes
test('removing regex pattern (with other keys) is supported', function (t) {
  t.plan(2)
  var instance = bloomrun()
  var pattern = { say: 'something', to: /.*/ }
  instance.add(pattern)
  t.deepEqual(instance.lookup({ say: 'something', to: 'you' }), pattern)
  instance.remove(pattern)
  t.equal(instance.lookup({ say: 'something', to: 'you' }), null)
})

// Fails: expected null, got { to: /.*/ } to the final lookup
test('removing regex pattern (without other keys) is supported', function (t) {
  t.plan(2)
  var instance = bloomrun()
  var pattern = { to: /.*/ }
  instance.add(pattern)
  t.deepEqual(instance.lookup({ to: 'you' }), pattern)
  instance.remove(pattern)
  t.equal(instance.lookup({ to: 'you' }), null)
})
@mcollina
Copy link
Owner

Good find! Would you like to send a PR? We are all maxed out atm, and it might take a while otherwise.

mcollina added a commit that referenced this issue Jan 14, 2017
Closes #48, removing regex pattern without other keys is allowed
gabssnake added a commit to gabssnake/bloomrun that referenced this issue Jan 26, 2017
@vdakalov
Copy link
Contributor

Guys, please look at this

var pattern = { to: /.*/ }

The test is fail when I use /^[a-zA-Z0-9-.]+$/i as pattern.

Just replace pattern in test and execute it.

@mcollina
Copy link
Owner

Would you mind sending a PR with a fix?

also cc @StarpTech

@StarpTech
Copy link
Collaborator

Atm no capacity to fix that but I will have an eye on the PR.

vdakalov added a commit to vdakalov/bloomrun that referenced this issue Nov 27, 2018
vdakalov added a commit to vdakalov/bloomrun that referenced this issue Nov 27, 2018
mcollina pushed a commit that referenced this issue Dec 3, 2018
* Resolve issue #48

* fix tests, make scripts windows compatible
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

4 participants