Skip to content

Feature/50 fix gitlab pathmatch 3 0 0 #52

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

Feature/50 fix gitlab pathmatch 3 0 0 #52

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
145b0c9
test: added default fixtures and github specific fixtures
Sep 27, 2019
305939f
test: added refactered tests for gists
Sep 27, 2019
fa87af7
fix: updated pathmatch for gitlab
Sep 27, 2019
b473c55
test: added basic test for ._fill() method
Sep 27, 2019
0aea712
test: added ignore; for 100% testing (this seems wonky)
Sep 28, 2019
a03d51e
test: added refactered tests for bitbucket
Sep 28, 2019
4693b9c
test: moved all github url tests together
Sep 28, 2019
d04239b
test: removed unused testing structure
Sep 28, 2019
af4835c
test: added script to get coverage report
Sep 28, 2019
f4f05cb
fix: update pathmatch for gitlab
claudiahdz Oct 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion git-host-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var gitHosts = module.exports = {
'bugstemplate': 'https://{domain}/{user}/{project}/issues',
'httpstemplate': 'git+https://{auth@}{domain}/{user}/{projectPath}.git{#committish}',
'tarballtemplate': 'https://{domain}/{user}/{project}/repository/archive.tar.gz?ref={committish}',
'pathmatch': /^[/]([^/]+)[/](.+?)(?:[.]git|[/])?$/
'pathmatch': /^[/]([^/]+)[/](?!.*\/-\/)(.+?)(?:[.]git|[/])?$/
Copy link

@lpinca lpinca Oct 1, 2019

Choose a reason for hiding this comment

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

I think this will also incorrectly match URLs like https://gitlab.com/user/foo/bar/baz/repository/archive.tar.gz

},
gist: {
'protocols': [ 'git', 'git+ssh', 'git+https', 'ssh', 'https' ],
Expand Down
1 change: 1 addition & 0 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function fromUrl (giturl, opts) {
var pathmatch = gitHostInfo.pathmatch
var matched = parsed.path.match(pathmatch)
if (!matched) return
/* istanbul ignore else */
Copy link
Contributor

@isaacs isaacs Sep 28, 2019

Choose a reason for hiding this comment

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

Could this be tested with a url where the host matches, but the path isn't a match? Like https://github.com/isaacs?

Possibly I'm misreading this bit, and it is actually just a safety guard that never hits. But in that case, it'd be preferable to make that explicit.

/*istanbul ignore if */
if (matched[1] === null || matched[1] === undefined) {
  throw new Error('this should be impossible')
}

if (matched[1] !== null && matched[1] !== undefined) {
user = decodeURIComponent(matched[1].replace(/^:/, ''))
}
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"postrelease": "npm publish && git push --follow-tags",
"pretest": "standard",
"release": "standard-version -s",
"test:coverage": "tap --coverage-report=html -J --100 --no-esm test/*.js",
"test": "tap -J --100 --no-esm test/*.js"
},
"dependencies": {
Expand Down
1 change: 1 addition & 0 deletions test/basic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var test = require('tap').test

test('basic', function (t) {
const h = HostedGit.fromUrl('github:user/project')
t.equal(h._fill(), undefined)
t.is(h.constructor, HostedGit)
t.is(h.constructor.name, 'GitHost')
t.is(HostedGit.fromUrl('https://google.com'), undefined, 'null on failure')
Expand Down
27 changes: 0 additions & 27 deletions test/bitbucket-https-with-embedded-auth.js
View file
Open in desktop

This file was deleted.

306 changes: 283 additions & 23 deletions test/bitbucket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,292 @@
var HostedGit = require('../index')
var test = require('tap').test

test('fromUrl(bitbucket url)', function (t) {
function verify (host, label, branch) {
var showLabel = function (label, fn) { return label + ' -> ' + fn }

var testFixtures = function (t, params, fixtures) {
for (var i = 0; i < fixtures.length; ++i) {
var fixture = fixtures[i]

var host = fixture.host(params)
var hostinfo = HostedGit.fromUrl(host)
var hash = branch ? '#' + branch : ''
t.ok(hostinfo, label)
if (!hostinfo) return
t.is(hostinfo.https(), 'git+https://bitbucket.org/111/222.git' + hash, label + ' -> https')
t.is(hostinfo.browse(), 'https://bitbucket.org/111/222' + (branch ? '/src/' + branch : ''), label + ' -> browse')
t.is(hostinfo.browse(''), 'https://bitbucket.org/111/222/src/' + (branch || 'master') + '/', label + ' -> browse(path)')
t.is(hostinfo.browse('C'), 'https://bitbucket.org/111/222/src/' + (branch || 'master') + '/C', label + ' -> browse(path)')
t.is(hostinfo.browse('C/D'), 'https://bitbucket.org/111/222/src/' + (branch || 'master') + '/C/D', label + ' -> browse(path)')
t.is(hostinfo.browse('C', 'A'), 'https://bitbucket.org/111/222/src/' + (branch || 'master') + '/C#a', label + ' -> browse(path, fragment)')
t.is(hostinfo.browse('C/D', 'A'), 'https://bitbucket.org/111/222/src/' + (branch || 'master') + '/C/D#a', label + ' -> browse(path, fragment)')
t.is(hostinfo.docs(), 'https://bitbucket.org/111/222' + (branch ? '/src/' + branch : '') + '#readme', label + ' -> docs')
t.is(hostinfo.ssh(), '[email protected]:111/222.git' + hash, label + ' -> ssh')
t.is(hostinfo.sshurl(), 'git+ssh://[email protected]/111/222.git' + hash, label + ' -> sshurl')
t.is(hostinfo.shortcut(), 'bitbucket:111/222' + hash, label + ' -> shortcut')
t.is(hostinfo.file(''), 'https://bitbucket.org/111/222/raw/' + (branch || 'master') + '/', label + ' -> file')
t.is(hostinfo.file('C'), 'https://bitbucket.org/111/222/raw/' + (branch || 'master') + '/C', label + ' -> file')
t.is(hostinfo.file('C/D'), 'https://bitbucket.org/111/222/raw/' + (branch || 'master') + '/C/D', label + ' -> file')
t.is(hostinfo.tarball(), 'https://bitbucket.org/111/222/get/' + (branch || 'master') + '.tar.gz', label + ' -> tarball')
t.is(hostinfo.tarball({ noCommittish: true }), 'https://bitbucket.org/111/222/get/' + (branch || 'master') + '.tar.gz', label + ' -> tarball')

// INFO: fromUrl should return `undefined` from fixture input
if (fixture.isUndefined) {
t.test('input results in undefined', function (tt) {
tt.is(hostinfo, undefined)
tt.end()
})
break
}

t.test('hostinfo.https', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url + '#' + params.branch
: url
}
tt.is(
hostinfo.https(),
expected('git+https://bitbucket.org/some-owner/some-repo.git', fixture.hasBranch, fixture.hasGroup),
showLabel(fixture.label, 'https')
)
// INFO: not using `expected` because with `{noCommittish: true}` the output is always the same
tt.is(
hostinfo.https({ noCommittish: true }),
'git+https://bitbucket.org/some-owner/some-repo.git',
showLabel(fixture.label, 'https({ noCommittish: true })')
)
tt.is(
hostinfo.https({ noGitPlus: true }),
expected('https://bitbucket.org/some-owner/some-repo.git', fixture.hasBranch),
showLabel(fixture.label, 'https({ noGitPlus: true })')
)
tt.end()
})
t.test('hostinfo.browse', function (tt) {
var expected = function (url, hasBranch) {
if (hasBranch) {
if (url.indexOf('master') === -1) {
return url + '/src/' + params.branch
} else {
return url.replace(/master/gi, params.branch)
}
}
return url
}
tt.is(
hostinfo.browse(),
expected('https://bitbucket.org/some-owner/some-repo', fixture.hasBranch),
showLabel(fixture.label, 'browse')
)
tt.is(
hostinfo.browse(''),
expected('https://bitbucket.org/some-owner/some-repo/src/master/', fixture.hasBranch),
showLabel(fixture.label, "browse('')")
)
tt.is(
hostinfo.browse('C'),
expected('https://bitbucket.org/some-owner/some-repo/src/master/C', fixture.hasBranch),
showLabel(fixture.label, "browse('C')")
)
tt.is(
hostinfo.browse('C/D'),
expected('https://bitbucket.org/some-owner/some-repo/src/master/C/D', fixture.hasBranch),
showLabel(fixture.label, "browse('C/D')")
)
tt.is(
hostinfo.browse('C', 'A'),
expected('https://bitbucket.org/some-owner/some-repo/src/master/C#a', fixture.hasBranch),
showLabel(fixture.label, "browse('C', 'A')")
)
tt.is(
hostinfo.browse('C/D', 'A'),
expected('https://bitbucket.org/some-owner/some-repo/src/master/C/D#a', fixture.hasBranch),
showLabel(fixture.label, "browse('C/D', 'A')")
)
tt.end()
})
t.test('hostinfo.docs', function (tt) {
var expected = function (url, hasBranch) {
if (hasBranch) {
var splitUrl = url.split('#')
return splitUrl[0] + '/src/' + params.branch + '#' + splitUrl[1]
}
return url
}
tt.is(
hostinfo.docs(),
expected('https://bitbucket.org/some-owner/some-repo#readme', fixture.hasBranch),
showLabel(fixture.label, 'docs')
)
tt.is(
hostinfo.docs({ noCommittish: true }),
// INFO: not using `expected` because with `{noCommittish: true}` the output is always the same
'https://bitbucket.org/some-owner/some-repo#readme',
showLabel(fixture.label, 'docs({ noCommittish: true })')
)
tt.is(
hostinfo.docs({ noGitPlus: true }),
expected('https://bitbucket.org/some-owner/some-repo#readme', fixture.hasBranch),
showLabel(fixture.label, 'docs({ noGitPlus: true })')
)
tt.end()
})
t.test('hostinfo.ssh', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url + '#' + params.branch
: url
}
tt.is(
hostinfo.ssh(),
expected('[email protected]:some-owner/some-repo.git', fixture.hasBranch),
showLabel(fixture.label, 'ssh')
)
tt.is(
hostinfo.ssh({ noCommittish: true }),
// INFO: not using `expected` because with `{noCommittish: true}` the output is always the same
'[email protected]:some-owner/some-repo.git',
showLabel(fixture.label, 'ssh({ noCommittish: true })')
)
tt.is(
hostinfo.ssh({ noGitPlus: true }),
expected('[email protected]:some-owner/some-repo.git', fixture.hasBranch),
showLabel(fixture.label, 'ssh({ noGitPlus: true })')
)
tt.end()
})
t.test('hostinfo.sshurl', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url + '#' + params.branch
: url
}
tt.is(
hostinfo.sshurl(),
expected('git+ssh://[email protected]/some-owner/some-repo.git', fixture.hasBranch),
showLabel(fixture.label, 'sshurl')
)
tt.is(
hostinfo.sshurl({ noCommittish: true }),
// INFO: not using `expected` because with `{noCommittish: true}` the output is always the same
'git+ssh://[email protected]/some-owner/some-repo.git',
showLabel(fixture.label, 'sshurl({ noCommittish: true })')
)
tt.is(
hostinfo.sshurl({ noGitPlus: true }),
expected('ssh://[email protected]/some-owner/some-repo.git', fixture.hasBranch),
showLabel(fixture.label, 'sshurl({ noGitPlus: true })')
)
tt.end()
})
t.test('hostinfo.shortcut', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url + '#' + params.branch
: url
}
tt.is(
hostinfo.shortcut(),
expected('bitbucket:some-owner/some-repo', fixture.hasBranch),
showLabel(fixture.label, 'shortcut')
)
tt.is(
hostinfo.shortcut({ noCommittish: true }),
// INFO: not using `expected` because with `{noCommittish: true}` the output is always the same
'bitbucket:some-owner/some-repo',
showLabel(fixture.label, 'shortcut({ noCommittish: true })')
)
tt.is(
hostinfo.shortcut({ noGitPlus: true }),
expected('bitbucket:some-owner/some-repo', fixture.hasBranch),
showLabel(fixture.label, 'shortcut({ noGitPlus: true })')
)
tt.end()
})
t.test('hostinfo.file', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url.replace(/master/gi, params.branch)
: url
}
tt.is(
hostinfo.file(),
expected('https://bitbucket.org/some-owner/some-repo/raw/master/', fixture.hasBranch),
showLabel(fixture.label, 'file')
)
tt.is(
hostinfo.file('C'),
expected('https://bitbucket.org/some-owner/some-repo/raw/master/C', fixture.hasBranch),
showLabel(fixture.label, "file('C')")
)
tt.is(
hostinfo.file('C/D'),
expected('https://bitbucket.org/some-owner/some-repo/raw/master/C/D', fixture.hasBranch),
showLabel(fixture.label, "file('C/D')")
)
tt.end()
})
t.test('hostinfo.tarball', function (tt) {
var expected = function (url, hasBranch) {
return (hasBranch)
? url.replace(/master/gi, params.branch)
: url
}
tt.is(
hostinfo.tarball(),
expected('https://bitbucket.org/some-owner/some-repo/get/master.tar.gz', fixture.hasBranch),
showLabel(fixture.label, 'tarball')
)
tt.is(
hostinfo.tarball({ noCommittish: true }),
expected('https://bitbucket.org/some-owner/some-repo/get/master.tar.gz', fixture.hasBranch),
showLabel(fixture.label, 'tarball({ noCommittish: true })')
)
tt.is(
hostinfo.tarball({ noGitPlus: true }),
expected('https://bitbucket.org/some-owner/some-repo/get/master.tar.gz', fixture.hasBranch),
showLabel(fixture.label, 'tarball({ noGitPlus: true })')
)
tt.end()
})
}
}

test('fromUrl(bitbucket url)', function (t) {
var fixtures = require('./fixtures/bitbucket')
// var gitlabFixtures = require('./fixtures/bitbucket')
// var collectedFixtures = [].concat(fixtures, gitlabFixtures)

t.test('domain: bitbucket.org', function (tt) {
var params = {
domain: 'bitbucket.org',
shortname: 'bitbucket',
label: 'bitbucket',
owner: 'some-owner',
project: 'some-repo',
branch: 'feature-branch'
}
testFixtures(tt, params, fixtures)
tt.end()
})

t.test('domain: www.bitbucket.org', function (tt) {
var params = {
domain: 'www.bitbucket.org',
shortname: 'bitbucket',
label: 'bitbucket',
owner: 'some-owner',
project: 'some-repo',
branch: 'feature-branch'
}
testFixtures(tt, params, fixtures)
tt.end()
})

require('./lib/standard-tests')(verify, 'bitbucket.org', 'bitbucket')
require('./lib/standard-tests')(verify, 'www.bitbucket.org', 'bitbucket')
t.test('Bitbucket HTTPS URLs with embedded auth', function (tt) {
tt.is(
HostedGit.fromUrl('https://user:[email protected]/user/repo.git').toString(),
'git+https://user:[email protected]/user/repo.git',
'credentials were included in URL'
)
tt.is(
HostedGit.fromUrl('https://user:[email protected]/user/repo').toString(),
'git+https://user:[email protected]/user/repo.git',
'credentials were included in URL'
)
tt.is(
HostedGit.fromUrl('git+https://user:[email protected]/user/repo.git').toString(),
'git+https://user:[email protected]/user/repo.git',
'credentials were included in URL'
)
tt.is(
HostedGit.fromUrl('git+https://user:[email protected]/user/repo').toString(),
'git+https://user:[email protected]/user/repo.git',
'credentials were included in URL'
)
tt.end()
})

t.end()
})
Loading