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.

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

feat: add env var to bypass hooks execution #96

Merged
merged 10 commits into from
Jan 4, 2024
Merged

feat: add env var to bypass hooks execution #96

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
79136a8
feat: add env var to bypass hooks execution
bhanufyi Sep 4, 2023
3d0ff4c
fix ci git init issues
bhanufyi Sep 5, 2023
c437eb7
fix(tests): replace with single afterEach
bhanufyi Sep 24, 2023
10f1ded
feat: export prepend script, reuse in tests
bhanufyi Sep 24, 2023
6ad9fe4
feat: use SKIP_SIMPLE_GIT_HOOKS=1 for bypassing
bhanufyi Sep 24, 2023
062a539
feat: use describe, reduce code duplication
bhanufyi Oct 6, 2023
d0b35e1
feat: add lodash.isequal and use isEqual instead of json stringify
bhanufyi Oct 6, 2023
b506dbd
refactor: improve tests by reducing duplication
bhanufyi Oct 6, 2023
095089d
docs: mention about 3rd party git clients
bhanufyi Oct 6, 2023
47dec20
refactor: use describe, it syntax for test
bhanufyi Oct 20, 2023
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
29 changes: 27 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ If you need multiple verbose commands per git hook, flexible configuration or au
# [Optional] These 2 steps can be skipped for non-husky users
git config core.hooksPath .git/hooks/
rm -rf .git/hooks

# Update ./git/hooks
npx simple-git-hooks
```
Expand Down Expand Up @@ -156,7 +156,32 @@ npm uninstall simple-git-hooks

You should use `--no-verify` option

https://bobbyhadz.com/blog/git-commit-skip-hooks#skip-git-commit-hooks
```sh
git commit -m "commit message" --no-verify # -n for shorthand
```

you can read more about it here https://bobbyhadz.com/blog/git-commit-skip-hooks#skip-git-commit-hooks


If you need to bypass hooks for multiple Git operations, setting the SKIP_SIMPLE_GIT_HOOKS environment variable can be more convenient. Once set, all subsequent Git operations in the same terminal session will bypass the associated hooks.
Copy link
Owner

Choose a reason for hiding this comment

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

It's best to also include use cases. I've seen people struggle with skipping git hooks when using 3rd party git clients

Normally (from the terminal) you can bypass hooks by using --skip-hooks option if im not mistaken :-)

Copy link
Owner

Choose a reason for hiding this comment

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

Like so:

Using with 3rd party clients:

...


```sh
# Set the environment variable
export SKIP_SIMPLE_GIT_HOOKS=1

# Subsequent Git commands will skip the hooks
git add .
git commit -m "commit message" # pre-commit hooks are bypassed
git push origin main # pre-push hooks are bypassed
```

### Skipping Hooks in 3rd party git clients

If your client provides a toggle to skip Git hooks, you can utilize it to bypass the hooks. For instance, in VSCode, you can toggle git.allowNoVerifyCommit in the settings.

If you have the option to set arguments or environment variables, you can use the --no-verify option or the SKIP_SIMPLE_GIT_HOOKS environment variable.

If these options are not available, you may need to resort to using the terminal for skipping hooks.

### When migrating from `husky` git hooks are not running

Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"clean-publish": "^4.2.0",
"eslint": "^7.19.0",
"jest": "^26.6.3",
"lint-staged": "^10.5.4"
"lint-staged": "^10.5.4",
"lodash.isequal": "^4.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this? I saw no usage?

}
}
10 changes: 9 additions & 1 deletion simple-git-hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ const VALID_GIT_HOOKS = [

const VALID_OPTIONS = ['preserveUnused']

const PREPEND_SCRIPT =
"#!/bin/sh\n\n" +
'if [ "$SKIP_SIMPLE_GIT_HOOKS" = "1" ]; then\n' +
' echo "[INFO] SKIP_SIMPLE_GIT_HOOKS is set to 1, skipping hook."\n' +
" exit 0\n" +
"fi\n\n";

/**
* Recursively gets the .git folder path from provided directory
* @param {string} directory
Expand Down Expand Up @@ -178,7 +185,7 @@ function _setHook(hook, command, projectRoot=process.cwd()) {
return
}

const hookCommand = "#!/bin/sh\n" + command
const hookCommand = PREPEND_SCRIPT + command
const hookDirectory = gitRoot + '/hooks/'
const hookPath = path.normalize(hookDirectory + hook)

Expand Down Expand Up @@ -361,4 +368,5 @@ module.exports = {
getProjectRootDirectoryFromNodeModules,
getGitProjectRoot,
removeHooks,
PREPEND_SCRIPT
}
155 changes: 121 additions & 34 deletions simple-git-hooks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const fs = require('fs')
const spc = require("./simple-git-hooks");
const path = require("path")
const { execSync } = require('child_process');
const isEqual = require('lodash.isequal');

const { version: packageVersion } = require('./package.json');

Expand Down Expand Up @@ -88,6 +90,8 @@ const projectWithCustomConfigurationFilePath = path.normalize(path.join(testsFol
const projectWithIncorrectConfigurationInPackageJson = path.normalize(path.join(testsFolder, 'project_with_incorrect_configuration_in_package_json'))
const projectWithoutConfiguration = path.normalize(path.join(testsFolder, 'project_without_configuration'))

const TEST_SCRIPT = `${spc.PREPEND_SCRIPT}exit 1`;

/**
* Creates .git/hooks dir from root
* @param {string} root
Expand Down Expand Up @@ -126,108 +130,122 @@ function getInstalledGitHooks(hooksDir) {
return result
}

afterEach(() => {
[
projectWithConfigurationInAlternativeSeparateJsPath,
projectWithConfigurationInAlternativeSeparateCjsPath,
projectWithConfigurationInSeparateCjsPath,
projectWithConfigurationInSeparateJsPath,
projectWithConfigurationInAlternativeSeparateJsonPath,
projectWithConfigurationInSeparateJsonPath,
projectWithConfigurationInPackageJsonPath,
projectWithIncorrectConfigurationInPackageJson,
projectWithoutConfiguration,
projectWithConfigurationInPackageJsonPath,
projectWithUnusedConfigurationInPackageJsonPath,
projectWithCustomConfigurationFilePath,
].forEach((testCase) => {
removeGitHooksFolder(testCase);
});
});


test('creates git hooks if configuration is correct from .simple-git-hooks.js', () => {
createGitHooksFolder(projectWithConfigurationInAlternativeSeparateJsPath)

spc.setHooksFromConfig(projectWithConfigurationInAlternativeSeparateJsPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInAlternativeSeparateJsPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInAlternativeSeparateJsPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})


test('creates git hooks if configuration is correct from .simple-git-hooks.cjs', () => {
createGitHooksFolder(projectWithConfigurationInAlternativeSeparateCjsPath)

spc.setHooksFromConfig(projectWithConfigurationInAlternativeSeparateCjsPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInAlternativeSeparateCjsPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInAlternativeSeparateCjsPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})


test('creates git hooks if configuration is correct from simple-git-hooks.cjs', () => {
createGitHooksFolder(projectWithConfigurationInSeparateCjsPath)

spc.setHooksFromConfig(projectWithConfigurationInSeparateCjsPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInSeparateCjsPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInSeparateCjsPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can extract { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT } as variable too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, extracted into COMMON_GIT_HOOKS variable

})


test('creates git hooks if configuration is correct from simple-git-hooks.js', () => {
createGitHooksFolder(projectWithConfigurationInSeparateJsPath)

spc.setHooksFromConfig(projectWithConfigurationInSeparateJsPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInSeparateJsPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInSeparateJsPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})


test('creates git hooks if configuration is correct from .simple-git-hooks.json', () => {
createGitHooksFolder(projectWithConfigurationInAlternativeSeparateJsonPath)

spc.setHooksFromConfig(projectWithConfigurationInAlternativeSeparateJsonPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInAlternativeSeparateJsonPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInAlternativeSeparateJsonPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})


test('creates git hooks if configuration is correct from simple-git-hooks.json', () => {
createGitHooksFolder(projectWithConfigurationInSeparateJsonPath)

spc.setHooksFromConfig(projectWithConfigurationInSeparateJsonPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInSeparateJsonPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithConfigurationInSeparateJsonPath)
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})


test('creates git hooks if configuration is correct from package.json', () => {
createGitHooksFolder(projectWithConfigurationInPackageJsonPath)

spc.setHooksFromConfig(projectWithConfigurationInPackageJsonPath)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInPackageJsonPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`}))
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT })).toBe(true);

removeGitHooksFolder(projectWithConfigurationInPackageJsonPath)
})


test('fails to create git hooks if configuration contains bad git hooks', () => {
createGitHooksFolder(projectWithIncorrectConfigurationInPackageJson)

expect(() => spc.setHooksFromConfig(projectWithIncorrectConfigurationInPackageJson)).toThrow('[ERROR] Config was not in correct format. Please check git hooks or options name')

removeGitHooksFolder(projectWithIncorrectConfigurationInPackageJson)
})


test('fails to create git hooks if not configured', () => {
createGitHooksFolder(projectWithoutConfiguration)

expect(() => spc.setHooksFromConfig(projectWithoutConfiguration)).toThrow('[ERROR] Config was not found! Please add `.simple-git-hooks.js` or `simple-git-hooks.js` or `.simple-git-hooks.json` or `simple-git-hooks.json` or `simple-git-hooks` entry in package.json.')

removeGitHooksFolder(projectWithoutConfiguration)
})


test('removes git hooks', () => {
createGitHooksFolder(projectWithConfigurationInPackageJsonPath)

spc.setHooksFromConfig(projectWithConfigurationInPackageJsonPath)

let installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInPackageJsonPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`}))
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT })).toBe(true)

spc.removeHooks(projectWithConfigurationInPackageJsonPath)

installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithConfigurationInPackageJsonPath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({}))
expect(isEqual(installedHooks, {})).toBe(true);

removeGitHooksFolder(projectWithConfigurationInPackageJsonPath)
})


test('creates git hooks and removes unused git hooks', () => {
createGitHooksFolder(projectWithConfigurationInPackageJsonPath)

Expand All @@ -236,14 +254,13 @@ test('creates git hooks and removes unused git hooks', () => {
fs.writeFileSync(path.resolve(installedHooksDir, 'pre-push'), "# do nothing")

let installedHooks = getInstalledGitHooks(installedHooksDir);
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-push':'# do nothing'}))
expect(isEqual(installedHooks, { 'pre-push': '# do nothing' })).toBe(true)

spc.setHooksFromConfig(projectWithConfigurationInPackageJsonPath)

installedHooks = getInstalledGitHooks(installedHooksDir);
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`}))
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT })).toBe(true);

removeGitHooksFolder(projectWithConfigurationInPackageJsonPath)
})


Expand All @@ -256,16 +273,16 @@ test('creates git hooks and removes unused but preserves specific git hooks', ()
fs.writeFileSync(path.resolve(installedHooksDir, 'pre-push'), "# do nothing")

let installedHooks = getInstalledGitHooks(installedHooksDir);
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'commit-msg': '# do nothing', 'pre-push':'# do nothing'}))
expect(isEqual(installedHooks, { 'commit-msg': '# do nothing', 'pre-push': '# do nothing' })).toBe(true);

spc.setHooksFromConfig(projectWithUnusedConfigurationInPackageJsonPath)

installedHooks = getInstalledGitHooks(installedHooksDir);
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'commit-msg': '# do nothing', 'pre-commit':`#!/bin/sh\nexit 1`}))
expect(isEqual(installedHooks, { 'commit-msg': '# do nothing', 'pre-commit': TEST_SCRIPT })).toBe(true);

removeGitHooksFolder(projectWithUnusedConfigurationInPackageJsonPath)
})


test.each([
['npx', 'simple-git-hooks', './git-hooks.js'],
['node', require.resolve(`./cli`), './git-hooks.js'],
Expand All @@ -275,7 +292,77 @@ test.each([

spc.setHooksFromConfig(projectWithCustomConfigurationFilePath, args)
const installedHooks = getInstalledGitHooks(path.normalize(path.join(projectWithCustomConfigurationFilePath, '.git', 'hooks')))
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({'pre-commit':`#!/bin/sh\nexit 1`, 'pre-push':`#!/bin/sh\nexit 1`}))

removeGitHooksFolder(projectWithCustomConfigurationFilePath)
expect(JSON.stringify(installedHooks)).toBe(JSON.stringify({ 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT }))
expect(isEqual(installedHooks, { 'pre-commit': TEST_SCRIPT, 'pre-push': TEST_SCRIPT })).toBe(true);
})

describe('Tests for skipping git hooks using SKIP_SIMPLE_GIT_HOOKS env var', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

It's nice to see you taking a look to jest describe syntax here!

But it makes no sense if it is used only in your tests. Let me try to be more clear on what I meant in the previous review:

So, we have our test code setup like this:

// Get project root directory

test('getProjectRootDirectory returns correct dir in typical case:', () => {
    expect(spc.getProjectRootDirectoryFromNodeModules('var/my-project/node_modules/simple-git-hooks')).toBe('var/my-project')
})

test('getProjectRootDirectory returns correct dir when used with windows delimiters:', () => {
    expect(spc.getProjectRootDirectoryFromNodeModules('user\\allProjects\\project\\node_modules\\simple-git-hooks')).toBe('user/allProjects/project')
})

We can use describe to make it look like this:

describe('Get Project Root Directory tests', () => {
  it('returns correct dir in typical case:', () => {
    expect(spc.getProjectRootDirectoryFromNodeModules('var/my-project/node_modules/simple-git-hooks')).toBe('var/my-project')
})

it('returns correct dir when used with windows delimiters:', () => {
    expect(spc.getProjectRootDirectoryFromNodeModules('user\\allProjects\\project\\node_modules\\simple-git-hooks')).toBe('user/allProjects/project')
})
})

This way tests can be grouped, which allows us to see pretty output: here is an example if we do the manipulations above

Screenshot 2023-10-07 at 21 32 15

--

I propose two solutions:

  • refactor all the tests to use describe syntax
  • or simply leave your tests as is (do not use describe syntax) - I will do refactoring later :-)

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 have used describe, it syntax, let me if it needs further work


const GIT_USER_NAME = "github-actions";
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can do this in GitHub actions itself, do not see why we need some GH actions specific code in repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the tests, I'm performing commit git operation which requires to set username and email, so I have used random values, let me know if it can be done in a better way

const GIT_USER_EMAIL = "[email protected]";

const initializeGitRepository = (path) => {
execSync(
`git init \
&& git config user.name ${GIT_USER_NAME} \
&& git config user.email ${GIT_USER_EMAIL}`,
{ cwd: path }
);
};

const performTestCommit = (path, env = process.env) => {
try {
execSync(
'git add . && git commit --allow-empty -m "Test commit" && git commit --allow-empty -am "Change commit msg"',
{
cwd: path,
env: env,
}
);
return false;
} catch (e) {
return true;
}
};

beforeEach(() => {
initializeGitRepository(projectWithConfigurationInPackageJsonPath);
createGitHooksFolder(projectWithConfigurationInPackageJsonPath);
spc.setHooksFromConfig(projectWithConfigurationInPackageJsonPath);
});

afterEach(() => {
delete process.env.SKIP_SIMPLE_GIT_HOOKS;
});

const expectCommitToSucceed = (path) => {
const errorOccurred = performTestCommit(path);
expect(errorOccurred).toBe(false);
};

const expectCommitToFail = (path) => {
const errorOccurred = performTestCommit(path);
expect(errorOccurred).toBe(true);
};

test('commits successfully when SKIP_SIMPLE_GIT_HOOKS is set to "1"', () => {
process.env.SKIP_SIMPLE_GIT_HOOKS = "1";
expectCommitToSucceed(projectWithConfigurationInPackageJsonPath);
});

test("commit fails when SKIP_SIMPLE_GIT_HOOKS is not set", () => {
expectCommitToFail(projectWithConfigurationInPackageJsonPath);
});

test('commit fails when SKIP_SIMPLE_GIT_HOOKS is set to "0"', () => {
process.env.SKIP_SIMPLE_GIT_HOOKS = "0";
expectCommitToFail(projectWithConfigurationInPackageJsonPath);
});

test("commit fails when SKIP_SIMPLE_GIT_HOOKS is set to a random string", () => {
process.env.SKIP_SIMPLE_GIT_HOOKS = "simple-git-hooks";
expectCommitToFail(projectWithConfigurationInPackageJsonPath);
});

});