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

Support fetching without the --progress option #1067

Merged
merged 1 commit into from Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Expand Up @@ -91,6 +91,10 @@ When Git 2.18 or higher is not in your PATH, falls back to the REST API to downl
# Default: false
fetch-tags: ''

# Whether to show progress status output when fetching.
# Default: true
show-progress: ''

# Whether to download Git-LFS files
# Default: false
lfs: ''
Expand Down
1 change: 1 addition & 0 deletions __test__/git-auth-helper.test.ts
Expand Up @@ -806,6 +806,7 @@ async function setup(testName: string): Promise<void> {
sparseCheckoutConeMode: true,
fetchDepth: 1,
fetchTags: false,
showProgress: true,
lfs: false,
submodules: false,
nestedSubmodules: false,
Expand Down
120 changes: 116 additions & 4 deletions __test__/git-command-manager.test.ts
Expand Up @@ -135,7 +135,6 @@ describe('Test fetchDepth and fetchTags options', () => {
'protocol.version=2',
'fetch',
'--prune',
'--progress',
'--no-recurse-submodules',
'--filter=filterValue',
'origin',
Expand Down Expand Up @@ -174,7 +173,6 @@ describe('Test fetchDepth and fetchTags options', () => {
'fetch',
'--no-tags',
'--prune',
'--progress',
'--no-recurse-submodules',
'--filter=filterValue',
'origin',
Expand Down Expand Up @@ -213,7 +211,6 @@ describe('Test fetchDepth and fetchTags options', () => {
'fetch',
'--no-tags',
'--prune',
'--progress',
'--no-recurse-submodules',
'--filter=filterValue',
'--depth=1',
Expand Down Expand Up @@ -252,7 +249,6 @@ describe('Test fetchDepth and fetchTags options', () => {
'protocol.version=2',
'fetch',
'--prune',
'--progress',
'--no-recurse-submodules',
'--filter=filterValue',
'--depth=1',
Expand All @@ -263,4 +259,120 @@ describe('Test fetchDepth and fetchTags options', () => {
expect.any(Object)
)
})

it('should call execGit with the correct arguments when showProgress is true', async () => {
jest.spyOn(exec, 'exec').mockImplementation(mockExec)

const workingDirectory = 'test'
const lfs = false
const doSparseCheckout = false
git = await commandManager.createCommandManager(
workingDirectory,
lfs,
doSparseCheckout
)
const refSpec = ['refspec1', 'refspec2']
const options = {
filter: 'filterValue',
showProgress: true
}

await git.fetch(refSpec, options)

expect(mockExec).toHaveBeenCalledWith(
expect.any(String),
[
'-c',
'protocol.version=2',
'fetch',
'--no-tags',
'--prune',
'--no-recurse-submodules',
'--progress',
'--filter=filterValue',
'origin',
'refspec1',
'refspec2'
],
expect.any(Object)
)
})

it('should call execGit with the correct arguments when fetchDepth is 42 and showProgress is true', async () => {
jest.spyOn(exec, 'exec').mockImplementation(mockExec)

const workingDirectory = 'test'
const lfs = false
const doSparseCheckout = false
git = await commandManager.createCommandManager(
workingDirectory,
lfs,
doSparseCheckout
)
const refSpec = ['refspec1', 'refspec2']
const options = {
filter: 'filterValue',
fetchDepth: 42,
showProgress: true
}

await git.fetch(refSpec, options)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be nice to merge these options onto the default option values here so the assertions become more like real world examples. (This is just an idea for a separate PR though.)


expect(mockExec).toHaveBeenCalledWith(
expect.any(String),
[
'-c',
'protocol.version=2',
'fetch',
'--no-tags',
'--prune',
'--no-recurse-submodules',
'--progress',
'--filter=filterValue',
'--depth=42',
'origin',
'refspec1',
'refspec2'
],
expect.any(Object)
)
})

it('should call execGit with the correct arguments when fetchTags is true and showProgress is true', async () => {
jest.spyOn(exec, 'exec').mockImplementation(mockExec)

const workingDirectory = 'test'
const lfs = false
const doSparseCheckout = false
git = await commandManager.createCommandManager(
workingDirectory,
lfs,
doSparseCheckout
)
const refSpec = ['refspec1', 'refspec2']
const options = {
filter: 'filterValue',
fetchTags: true,
showProgress: true
}

await git.fetch(refSpec, options)

expect(mockExec).toHaveBeenCalledWith(
expect.any(String),
[
'-c',
'protocol.version=2',
'fetch',
'--prune',
'--no-recurse-submodules',
'--progress',
'--filter=filterValue',
'origin',
'refspec1',
'refspec2'
],
expect.any(Object)
)
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be happy to remove some of the test coverage if it's considered too much. I was trying to follow the example set in #579 .

Copy link

@yeikel yeikel Aug 31, 2023

Choose a reason for hiding this comment

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

Too much coverage is rarely a problem. Thank you for the detailed effort!

})
1 change: 1 addition & 0 deletions __test__/input-helper.test.ts
Expand Up @@ -83,6 +83,7 @@ describe('input-helper tests', () => {
expect(settings.sparseCheckoutConeMode).toBe(true)
expect(settings.fetchDepth).toBe(1)
expect(settings.fetchTags).toBe(false)
expect(settings.showProgress).toBe(true)
expect(settings.lfs).toBe(false)
expect(settings.ref).toBe('refs/heads/some-ref')
expect(settings.repositoryName).toBe('some-repo')
Expand Down
3 changes: 3 additions & 0 deletions action.yml
Expand Up @@ -68,6 +68,9 @@ inputs:
fetch-tags:
description: 'Whether to fetch tags, even if fetch-depth > 0.'
default: false
show-progress:
description: 'Whether to show progress status output when fetching.'
default: true
lfs:
description: 'Whether to download Git-LFS files'
default: false
Expand Down
9 changes: 8 additions & 1 deletion dist/index.js
Expand Up @@ -640,7 +640,10 @@ class GitCommandManager {
if (!refSpec.some(x => x === refHelper.tagsRefSpec) && !options.fetchTags) {
args.push('--no-tags');
}
args.push('--prune', '--progress', '--no-recurse-submodules');
args.push('--prune', '--no-recurse-submodules');
if (options.showProgress) {
args.push('--progress');
}
if (options.filter) {
args.push(`--filter=${options.filter}`);
}
Expand Down Expand Up @@ -1739,6 +1742,10 @@ function getInputs() {
result.fetchTags =
(core.getInput('fetch-tags') || 'false').toUpperCase() === 'TRUE';
core.debug(`fetch tags = ${result.fetchTags}`);
// Show fetch progress
result.showProgress =
(core.getInput('show-progress') || 'true').toUpperCase() === 'TRUE';
core.debug(`show progress = ${result.showProgress}`);
// LFS
result.lfs = (core.getInput('lfs') || 'false').toUpperCase() === 'TRUE';
core.debug(`lfs = ${result.lfs}`);
Expand Down
13 changes: 11 additions & 2 deletions src/git-command-manager.ts
Expand Up @@ -34,6 +34,7 @@ export interface IGitCommandManager {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
}
): Promise<void>
getDefaultBranch(repositoryUrl: string): Promise<string>
Expand Down Expand Up @@ -241,14 +242,22 @@ class GitCommandManager {

async fetch(
refSpec: string[],
options: {filter?: string; fetchDepth?: number; fetchTags?: boolean}
options: {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
}
): Promise<void> {
const args = ['-c', 'protocol.version=2', 'fetch']
if (!refSpec.some(x => x === refHelper.tagsRefSpec) && !options.fetchTags) {
args.push('--no-tags')
}

args.push('--prune', '--progress', '--no-recurse-submodules')
args.push('--prune', '--no-recurse-submodules')
if (options.showProgress) {
args.push('--progress')
}

if (options.filter) {
args.push(`--filter=${options.filter}`)
Expand Down
1 change: 1 addition & 0 deletions src/git-source-provider.ts
Expand Up @@ -157,6 +157,7 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
} = {}
if (settings.sparseCheckout) fetchOptions.filter = 'blob:none'
if (settings.fetchDepth <= 0) {
Expand Down
5 changes: 5 additions & 0 deletions src/git-source-settings.ts
Expand Up @@ -49,6 +49,11 @@ export interface IGitSourceSettings {
*/
fetchTags: boolean

/**
* Indicates whether to use the --progress option when fetching
*/
showProgress: boolean

/**
* Indicates whether to fetch LFS objects
*/
Expand Down
5 changes: 5 additions & 0 deletions src/input-helper.ts
Expand Up @@ -105,6 +105,11 @@ export async function getInputs(): Promise<IGitSourceSettings> {
(core.getInput('fetch-tags') || 'false').toUpperCase() === 'TRUE'
core.debug(`fetch tags = ${result.fetchTags}`)

// Show fetch progress
result.showProgress =
(core.getInput('show-progress') || 'true').toUpperCase() === 'TRUE'
core.debug(`show progress = ${result.showProgress}`)

// LFS
result.lfs = (core.getInput('lfs') || 'false').toUpperCase() === 'TRUE'
core.debug(`lfs = ${result.lfs}`)
Expand Down