Skip to content

Commit

Permalink
Written tests for contains, in_array and getDomainFromUrl.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mario Basic committed Jun 30, 2015
1 parent 12b153b commit 9598a53
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 1 deletion.
2 changes: 1 addition & 1 deletion assets/js/helpers/contains.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function contains(line, list) {
if(cleanLine === '') continue;

// If line contains the clean line return true
if (line.indexOf(cleanLine) > - 1) {
if (cleanLine.indexOf(line) > - 1) {

This comment has been minimized.

Sign in to view

Copy link
@dboczek

dboczek Jun 24, 2016

I think this code was ok before. Line can be:
https://example.com/aaa/bbb/ccc
and whitelist

https://example.com
//other.domain.com

It should search for whitelisted line in url not opposite otherwise we have to put all urls into whitelist instead on domain only.

The problem is that tests are not correct. See my other comments.

This comment has been minimized.

Sign in to view

Copy link
@alanhamlett

alanhamlett Jun 29, 2016

Member

It's correct, but the variable names are not very clear. I've fixed it and will push up the new code shortly.

This comment has been minimized.

Sign in to view

Copy link
@alanhamlett

alanhamlett Jun 29, 2016

Member

Oh nevermind, you are right. I'll change the code so your tests pass.
return true;
}
}
Expand Down
14 changes: 14 additions & 0 deletions tests/helpers/contains.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,18 @@ describe('contains', function() {
it('should be a function', function() {
expect(contains).to.be.a('function');
});

it('should find the line and return true', function() {

This comment has been minimized.

Sign in to view

Copy link
@dboczek

dboczek Jun 24, 2016 

    it('should find the line and return true', function() {

        var list = "//example.com\n//test.com";

        expect(contains('http://example.com/path/to/page', list)).to.equal(true);
    });

    it('should not find the line and it should return false', function() {

        var list = "//example.com\n//test.com";

        expect(contains('http://example2.com/path/to/page'', list)).to.equal(false);
    });

If '.app2' could be URL still it should match with .app whitelist entry as it contains whitelist term.

var list = ".app\ntest.com";

expect(contains('.app', list)).to.equal(true);
});

it('should not find the line and it should return false', function() {

var list = ".app\ntest.com";

expect(contains('.app2', list)).to.equal(false);
});
});
9 changes: 9 additions & 0 deletions tests/helpers/getDomainFromUrl.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,13 @@ describe('getDomainFromUrl', function() {
it('should be a function', function() {
expect(getDomainFromUrl).to.be.a('function');
});

it('should return the domain', function() {
expect(getDomainFromUrl('http://google.com/something/very/secret')).to.equal('http://google.com');

expect(getDomainFromUrl('http://www.google.com/something/very/secret')).to.equal('http://www.google.com');

// This is not how it was imaged to work, but let's leave it here as a warning.
expect(getDomainFromUrl('google.com/something/very/secret')).to.equal('google.com//very');
});
});
8 changes: 8 additions & 0 deletions tests/helpers/in_array.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,12 @@ describe('in_array', function() {
it('should be a function', function() {
expect(in_array).to.be.a('function');
});

it('should find the needle and return true', function() {
expect(in_array('4', ['4', '3', '2', '1'])).to.equal(true);
});

it('should not find the needle and it should return false', function() {
expect(in_array('5', ['4', '3', '2', '1'])).to.equal(false);
});
});

0 comments on commit 9598a53

Please sign in to comment.