-
Notifications
You must be signed in to change notification settings - Fork 317
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 optimization] Configuration parameter to disable git unshallow #5291
Merged
+24
−1
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bd2d56a
Add env variable to disable git unshallow
Mariovido 879fdc5
Fix tests
Mariovido c754615
Change environment variable check
Mariovido 139a627
Rename env variable
Mariovido f8e5c6b
Merge branch 'master' into mario.vidal/git_unshallow
Mariovido ca1a39e
Reduce complexity by not adding to the config
Mariovido 7d42f82
Refactor test
Mariovido File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,8 @@ describe('git_metadata', () => { | |
| let isShallowRepositoryStub | ||
| let unshallowRepositoryStub | ||
|
|
||
| let isUnshallowEnabled | ||
|
|
||
| before(() => { | ||
| process.env.DD_API_KEY = 'api-key' | ||
| fs.writeFileSync(temporaryPackFile, '') | ||
|
|
@@ -47,6 +49,8 @@ describe('git_metadata', () => { | |
| isShallowRepositoryStub = sinon.stub().returns(false) | ||
| unshallowRepositoryStub = sinon.stub() | ||
|
|
||
| isUnshallowEnabled = true | ||
|
|
||
| generatePackFilesForCommitsStub = sinon.stub().returns([temporaryPackFile]) | ||
|
|
||
| gitMetadata = proxyquire('../../../../src/ci-visibility/exporters/git/git_metadata', { | ||
|
|
@@ -76,7 +80,8 @@ describe('git_metadata', () => { | |
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should unshallow if the repo is shallow and not every commit is in the backend', (done) => { | ||
|
|
@@ -94,7 +99,28 @@ describe('git_metadata', () => { | |
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should not unshallow if the parameter to enable unshallow is false', (done) => { | ||
| isUnshallowEnabled = false | ||
| const scope = nock('https://api.test.com') | ||
| .post('/api/v2/git/repository/search_commits') | ||
| .reply(200, JSON.stringify({ data: [] })) | ||
| .post('/api/v2/git/repository/search_commits') // calls a second time after unshallowing | ||
| .reply(200, JSON.stringify({ data: [] })) | ||
| .post('/api/v2/git/repository/packfile') | ||
| .reply(204) | ||
|
|
||
| isShallowRepositoryStub.returns(true) | ||
| gitMetadata.sendGitMetadata(new URL('https://api.test.com'), { isEvpProxy: false }, '', (err) => { | ||
| expect(unshallowRepositoryStub).not.to.have.been.called | ||
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should request to /api/v2/git/repository/search_commits and /api/v2/git/repository/packfile', (done) => { | ||
|
|
@@ -108,7 +134,8 @@ describe('git_metadata', () => { | |
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should not request to /api/v2/git/repository/packfile if the backend has the commit info', (done) => { | ||
|
|
@@ -126,7 +153,8 @@ describe('git_metadata', () => { | |
| expect(scope.isDone()).to.be.false | ||
| expect(scope.pendingMocks()).to.contain('POST https://api.test.com:443/api/v2/git/repository/packfile') | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fail and not continue if first query results in anything other than 200', (done) => { | ||
|
|
@@ -143,7 +171,8 @@ describe('git_metadata', () => { | |
| expect(scope.isDone()).to.be.false | ||
| expect(scope.pendingMocks()).to.contain('POST https://api.test.com:443/api/v2/git/repository/packfile') | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fail and not continue if the response are not correct commits', (done) => { | ||
|
|
@@ -159,7 +188,8 @@ describe('git_metadata', () => { | |
| expect(scope.isDone()).to.be.false | ||
| expect(scope.pendingMocks()).to.contain('POST https://api.test.com:443/api/v2/git/repository/packfile') | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fail and not continue if the response are badly formatted commits', (done) => { | ||
|
|
@@ -175,7 +205,8 @@ describe('git_metadata', () => { | |
| expect(scope.isDone()).to.be.false | ||
| expect(scope.pendingMocks()).to.contain('POST https://api.test.com:443/api/v2/git/repository/packfile') | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fail if the packfile request returns anything other than 204', (done) => { | ||
|
|
@@ -189,7 +220,8 @@ describe('git_metadata', () => { | |
| expect(err.message).to.contain('Could not upload packfiles: status code 502') | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fail if the getCommitsRevList fails because the repository is too big', (done) => { | ||
|
|
@@ -203,7 +235,8 @@ describe('git_metadata', () => { | |
| expect(err.message).to.contain('git rev-list failed') | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should fire a request per packfile', (done) => { | ||
|
|
@@ -230,7 +263,8 @@ describe('git_metadata', () => { | |
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| describe('validateGitRepositoryUrl', () => { | ||
|
|
@@ -305,7 +339,8 @@ describe('git_metadata', () => { | |
| expect(err.message).to.contain('Could not read "not-there"') | ||
| expect(scope.isDone()).to.be.false | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should not crash if generatePackFiles returns an empty array', (done) => { | ||
|
|
@@ -321,7 +356,8 @@ describe('git_metadata', () => { | |
| expect(err.message).to.contain('Failed to generate packfiles') | ||
| expect(scope.isDone()).to.be.false | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should not crash if git is missing', (done) => { | ||
|
|
@@ -340,7 +376,8 @@ describe('git_metadata', () => { | |
| expect(scope.isDone()).to.be.false | ||
| process.env.PATH = oldPath | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should retry if backend temporarily fails', (done) => { | ||
|
|
@@ -358,7 +395,8 @@ describe('git_metadata', () => { | |
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| done() | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should append evp proxy prefix if configured', (done) => { | ||
|
|
@@ -378,7 +416,8 @@ describe('git_metadata', () => { | |
| (err) => { | ||
| expect(err).to.be.null | ||
| expect(scope.isDone()).to.be.true | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
|
|
||
| it('should use the input repository url and not call getRepositoryUrl', (done) => { | ||
|
|
@@ -408,6 +447,7 @@ describe('git_metadata', () => { | |
| expect(repositoryUrl).to.equal('https://[email protected]') | ||
| done() | ||
| }) | ||
| }) | ||
| }, | ||
| isUnshallowEnabled) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this
DD_CIVISIBILITY_GIT_UNSHALLOW_ENABLEDso as to align with Java.Also: let me think if we need to go through the config changes. Maybe we can check
process.env.DD_CIVISIBILITY_GIT_UNSHALLOW_ENABLEDdirectly in the git code, as I see no advantage in adding it to the configThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed!