Skip to content

Commit dabbd44

Browse files
andypolsandypols
authored
Merge branch 'main' into add-proxy-healthcheck
2 parents 4216f18 + 93a1aaf commit dabbd44

File tree

6 files changed

+306
-28
lines changed

6 files changed

+306
-28
lines changed

‎package-lock.json‎

Lines changed: 37 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@
107107
"@vitejs/plugin-react": "^4.7.0",
108108
"chai": "^4.5.0",
109109
"chai-http": "^4.4.0",
110-
"cypress": "^14.5.4",
110+
"cypress": "^15.2.0",
111111
"eslint": "^8.57.1",
112112
"eslint-config-google": "^0.14.0",
113113
"eslint-config-prettier": "^10.1.8",

‎src/proxy/processors/push-action/scanDiff.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,10 @@ type Match = {
3535
const getDiffViolations = (diff: string, organization: string): Match[] | string | null => {
3636
// Commit diff is empty, i.e. '', null or undefined
3737
if (!diff) {
38-
console.log('No commit diff...');
39-
return 'No commit diff...';
38+
console.log('No commit diff found, but this may be legitimate (empty diff)');
39+
// Empty diff is not necessarily a violation - could be legitimate
40+
// (e.g., cherry-pick with no changes, reverts, etc.)
41+
return null;
4042
}
4143

4244
// Validation for configured block pattern(s) check...

‎test/integration/forcePush.integration.test.js‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
const path = require('path');
2+
const simpleGit = require('simple-git');
3+
const fs = require('fs').promises;
4+
const { Action } = require('../../src/proxy/actions');
5+
const { exec: getDiff } = require('../../src/proxy/processors/push-action/getDiff');
6+
const { exec: scanDiff } = require('../../src/proxy/processors/push-action/scanDiff');
7+
8+
const chai = require('chai');
9+
const expect = chai.expect;
10+
11+
describe('Force Push Integration Test', () => {
12+
let tempDir;
13+
let git;
14+
let initialCommitSHA;
15+
let rebasedCommitSHA;
16+
17+
before(async function () {
18+
this.timeout(10000); // eslint-disable-line no-invalid-this
19+
20+
tempDir = path.join(__dirname, '../temp-integration-repo');
21+
await fs.mkdir(tempDir, { recursive: true });
22+
git = simpleGit(tempDir);
23+
24+
await git.init();
25+
await git.addConfig('user.name', 'Test User');
26+
await git.addConfig('user.email', 'test@example.com');
27+
28+
// Create initial commit
29+
await fs.writeFile(path.join(tempDir, 'base.txt'), 'base content');
30+
await git.add('.');
31+
await git.commit('Initial commit');
32+
33+
// Create feature commit
34+
await fs.writeFile(path.join(tempDir, 'feature.txt'), 'feature content');
35+
await git.add('.');
36+
await git.commit('Add feature');
37+
38+
const log = await git.log();
39+
initialCommitSHA = log.latest.hash;
40+
41+
// Simulate rebase by amending commit (changes SHA)
42+
await git.commit(['--amend', '-m', 'Add feature (rebased)']);
43+
44+
const newLog = await git.log();
45+
rebasedCommitSHA = newLog.latest.hash;
46+
47+
console.log(`Initial SHA: ${initialCommitSHA}`);
48+
console.log(`Rebased SHA: ${rebasedCommitSHA}`);
49+
});
50+
51+
after(async () => {
52+
try {
53+
await fs.rmdir(tempDir, { recursive: true });
54+
} catch (e) {
55+
// Ignore cleanup errors
56+
}
57+
});
58+
59+
describe('Complete force push pipeline', () => {
60+
it('should handle valid diff after rebase scenario', async function () {
61+
this.timeout(5000); // eslint-disable-line no-invalid-this
62+
63+
// Create action simulating force push with valid SHAs that have actual changes
64+
const action = new Action(
65+
'valid-diff-integration',
66+
'push',
67+
'POST',
68+
Date.now(),
69+
'test/repo.git',
70+
);
71+
action.proxyGitPath = path.dirname(tempDir);
72+
action.repoName = path.basename(tempDir);
73+
74+
// Parent of initial commit to get actual diff content
75+
const parentSHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
76+
action.commitFrom = parentSHA;
77+
action.commitTo = rebasedCommitSHA;
78+
action.commitData = [
79+
{
80+
parent: parentSHA,
81+
commit: rebasedCommitSHA,
82+
message: 'Add feature (rebased)',
83+
author: 'Test User',
84+
},
85+
];
86+
87+
const afterGetDiff = await getDiff({}, action);
88+
expect(afterGetDiff.steps).to.have.length.greaterThan(0);
89+
90+
const diffStep = afterGetDiff.steps.find((s) => s.stepName === 'diff');
91+
expect(diffStep).to.exist;
92+
expect(diffStep.error).to.be.false;
93+
expect(diffStep.content).to.be.a('string');
94+
expect(diffStep.content.length).to.be.greaterThan(0);
95+
96+
const afterScanDiff = await scanDiff({}, afterGetDiff);
97+
const scanStep = afterScanDiff.steps.find((s) => s.stepName === 'scanDiff');
98+
99+
expect(scanStep).to.exist;
100+
expect(scanStep.error).to.be.false;
101+
});
102+
103+
it('should handle unreachable commit SHA error', async function () {
104+
this.timeout(5000); // eslint-disable-line no-invalid-this
105+
106+
// Invalid SHA to trigger error
107+
const action = new Action(
108+
'unreachable-sha-integration',
109+
'push',
110+
'POST',
111+
Date.now(),
112+
'test/repo.git',
113+
);
114+
action.proxyGitPath = path.dirname(tempDir);
115+
action.repoName = path.basename(tempDir);
116+
action.commitFrom = 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'; // Invalid SHA
117+
action.commitTo = rebasedCommitSHA;
118+
action.commitData = [
119+
{
120+
parent: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef',
121+
commit: rebasedCommitSHA,
122+
message: 'Add feature (rebased)',
123+
author: 'Test User',
124+
},
125+
];
126+
127+
const afterGetDiff = await getDiff({}, action);
128+
expect(afterGetDiff.steps).to.have.length.greaterThan(0);
129+
130+
const diffStep = afterGetDiff.steps.find((s) => s.stepName === 'diff');
131+
expect(diffStep).to.exist;
132+
expect(diffStep.error).to.be.true;
133+
expect(diffStep.errorMessage).to.be.a('string');
134+
expect(diffStep.errorMessage.length).to.be.greaterThan(0);
135+
expect(diffStep.errorMessage).to.satisfy(
136+
(msg) => msg.includes('fatal:') && msg.includes('Invalid revision range'),
137+
'Error message should contain git diff specific error for invalid SHA',
138+
);
139+
140+
// scanDiff should not block on missing diff due to error
141+
const afterScanDiff = await scanDiff({}, afterGetDiff);
142+
const scanStep = afterScanDiff.steps.find((s) => s.stepName === 'scanDiff');
143+
144+
expect(scanStep).to.exist;
145+
expect(scanStep.error).to.be.false;
146+
});
147+
148+
it('should handle missing diff step gracefully', async function () {
149+
const action = new Action(
150+
'missing-diff-integration',
151+
'push',
152+
'POST',
153+
Date.now(),
154+
'test/repo.git',
155+
);
156+
157+
const result = await scanDiff({}, action);
158+
159+
expect(result.steps).to.have.length(1);
160+
expect(result.steps[0].stepName).to.equal('scanDiff');
161+
expect(result.steps[0].error).to.be.false;
162+
});
163+
});
164+
});

‎test/processors/scanDiff.emptyDiff.test.js‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
const { Action } = require('../../src/proxy/actions');
2+
const { exec } = require('../../src/proxy/processors/push-action/scanDiff');
3+
4+
const chai = require('chai');
5+
const expect = chai.expect;
6+
7+
describe('scanDiff - Empty Diff Handling', () => {
8+
describe('Empty diff scenarios', () => {
9+
it('should allow empty diff (legitimate empty push)', async () => {
10+
const action = new Action('empty-diff-test', 'push', 'POST', Date.now(), 'test/repo.git');
11+
12+
// Simulate getDiff step with empty content
13+
const diffStep = { stepName: 'diff', content: '', error: false };
14+
action.steps = [diffStep];
15+
16+
const result = await exec({}, action);
17+
18+
expect(result.steps.length).to.equal(2); // diff step + scanDiff step
19+
expect(result.steps[1].error).to.be.false;
20+
expect(result.steps[1].errorMessage).to.be.null;
21+
});
22+
23+
it('should allow null diff', async () => {
24+
const action = new Action('null-diff-test', 'push', 'POST', Date.now(), 'test/repo.git');
25+
26+
// Simulate getDiff step with null content
27+
const diffStep = { stepName: 'diff', content: null, error: false };
28+
action.steps = [diffStep];
29+
30+
const result = await exec({}, action);
31+
32+
expect(result.steps.length).to.equal(2);
33+
expect(result.steps[1].error).to.be.false;
34+
expect(result.steps[1].errorMessage).to.be.null;
35+
});
36+
37+
it('should allow undefined diff', async () => {
38+
const action = new Action('undefined-diff-test', 'push', 'POST', Date.now(), 'test/repo.git');
39+
40+
// Simulate getDiff step with undefined content
41+
const diffStep = { stepName: 'diff', content: undefined, error: false };
42+
action.steps = [diffStep];
43+
44+
const result = await exec({}, action);
45+
46+
expect(result.steps.length).to.equal(2);
47+
expect(result.steps[1].error).to.be.false;
48+
expect(result.steps[1].errorMessage).to.be.null;
49+
});
50+
});
51+
52+
describe('Normal diff processing', () => {
53+
it('should process valid diff content without blocking', async () => {
54+
const action = new Action('valid-diff-test', 'push', 'POST', Date.now(), 'test/repo.git');
55+
action.project = 'test-org';
56+
57+
// Simulate normal diff content
58+
const normalDiff = `diff --git a/config.js b/config.js
59+
index 1234567..abcdefg 100644
60+
--- a/config.js
61+
+++ b/config.js
62+
@@ -1,3 +1,4 @@
63+
module.exports = {
64+
+ newFeature: true,
65+
database: "production"
66+
};`;
67+
68+
const diffStep = { stepName: 'diff', content: normalDiff, error: false };
69+
action.steps = [diffStep];
70+
71+
const result = await exec({}, action);
72+
73+
expect(result.steps[1].error).to.be.false;
74+
expect(result.steps[1].errorMessage).to.be.null;
75+
});
76+
});
77+
78+
describe('Error conditions', () => {
79+
it('should handle non-string diff content', async () => {
80+
const action = new Action('non-string-test', 'push', 'POST', Date.now(), 'test/repo.git');
81+
82+
const diffStep = { stepName: 'diff', content: 12345, error: false };
83+
action.steps = [diffStep];
84+
85+
const result = await exec({}, action);
86+
87+
expect(result.steps[1].error).to.be.true;
88+
expect(result.steps[1].errorMessage).to.include('non-string value');
89+
});
90+
});
91+
});

0 commit comments

Comments
 (0)