Skip to content

Commit 5fda254

Browse files
Copilotdata-douser
andauthored
fix: add .npmrc tests, fix pre-finalize.cmd ERRORLEVEL handling, remove accidentally committed .codeql-version
Agent-Logs-Url: https://github.com/advanced-security/codeql-sap-js/sessions/8906d85d-2830-47c2-b48a-447b735a4538 Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
1 parent 58c0137 commit 5fda254

File tree

2 files changed

+185
-3
lines changed

2 files changed

+185
-3
lines changed

extractors/cds/tools/test/src/packageManager/cacheInstaller.test.ts

Lines changed: 174 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,18 @@ import * as fs from 'fs';
33
import * as path from 'path';
44

55
import { CdsDependencyGraph, CdsProject } from '../../../src/cds/parser/types';
6-
import { cacheInstallDependencies } from '../../../src/packageManager';
6+
import {
7+
cacheInstallDependencies,
8+
copyNpmrcToCache,
9+
findNearestNpmrc,
10+
} from '../../../src/packageManager';
711

812
// Mock dependencies
913
jest.mock('fs', () => ({
14+
copyFileSync: jest.fn(),
1015
existsSync: jest.fn(),
11-
readFileSync: jest.fn(),
1216
mkdirSync: jest.fn(),
17+
readFileSync: jest.fn(),
1318
writeFileSync: jest.fn(),
1419
}));
1520

@@ -638,4 +643,171 @@ describe('installer', () => {
638643
expect(result.size).toBe(0);
639644
});
640645
});
646+
647+
describe('findNearestNpmrc', () => {
648+
beforeEach(() => {
649+
// Use actual path.dirname so the directory-walking loop works correctly
650+
(path.dirname as jest.Mock).mockImplementation(jest.requireActual('path').dirname);
651+
(fs.existsSync as jest.Mock).mockReturnValue(false);
652+
});
653+
654+
it('should return the .npmrc path when it exists in the start directory', () => {
655+
(path.resolve as jest.Mock).mockReturnValue('/project/src');
656+
(fs.existsSync as jest.Mock).mockImplementation((p: string) => p === '/project/src/.npmrc');
657+
658+
const result = findNearestNpmrc('/project/src');
659+
660+
expect(result).toBe('/project/src/.npmrc');
661+
});
662+
663+
it('should return the .npmrc path when it exists in a parent directory', () => {
664+
(path.resolve as jest.Mock).mockReturnValue('/project/src');
665+
(fs.existsSync as jest.Mock).mockImplementation((p: string) => p === '/project/.npmrc');
666+
667+
const result = findNearestNpmrc('/project/src');
668+
669+
expect(result).toBe('/project/.npmrc');
670+
});
671+
672+
it('should return undefined when no .npmrc exists in the directory tree', () => {
673+
(path.resolve as jest.Mock).mockReturnValue('/project/src');
674+
(fs.existsSync as jest.Mock).mockReturnValue(false);
675+
676+
const result = findNearestNpmrc('/project/src');
677+
678+
expect(result).toBeUndefined();
679+
});
680+
});
681+
682+
describe('copyNpmrcToCache', () => {
683+
beforeEach(() => {
684+
(path.dirname as jest.Mock).mockImplementation(jest.requireActual('path').dirname);
685+
(fs.existsSync as jest.Mock).mockReturnValue(false);
686+
(fs.copyFileSync as jest.Mock).mockReturnValue(undefined);
687+
});
688+
689+
it('should copy .npmrc to the cache directory when found', () => {
690+
(path.resolve as jest.Mock).mockReturnValue('/project');
691+
(fs.existsSync as jest.Mock).mockImplementation((p: string) => p === '/project/.npmrc');
692+
693+
copyNpmrcToCache('/cache/dir', '/project');
694+
695+
expect(fs.copyFileSync).toHaveBeenCalledWith('/project/.npmrc', '/cache/dir/.npmrc');
696+
});
697+
698+
it('should do nothing when no .npmrc is found', () => {
699+
(path.resolve as jest.Mock).mockReturnValue('/project');
700+
(fs.existsSync as jest.Mock).mockReturnValue(false);
701+
702+
copyNpmrcToCache('/cache/dir', '/project');
703+
704+
expect(fs.copyFileSync).not.toHaveBeenCalled();
705+
});
706+
707+
it('should log a warning and not throw when copyFileSync fails', () => {
708+
(path.resolve as jest.Mock).mockReturnValue('/project');
709+
(fs.existsSync as jest.Mock).mockImplementation((p: string) => p === '/project/.npmrc');
710+
(fs.copyFileSync as jest.Mock).mockImplementation(() => {
711+
throw new Error('Permission denied');
712+
});
713+
714+
expect(() => copyNpmrcToCache('/cache/dir', '/project')).not.toThrow();
715+
});
716+
});
717+
718+
describe('cacheInstallDependencies .npmrc propagation', () => {
719+
beforeEach(() => {
720+
(path.dirname as jest.Mock).mockImplementation(jest.requireActual('path').dirname);
721+
(fs.existsSync as jest.Mock).mockReturnValue(false);
722+
(fs.mkdirSync as jest.Mock).mockReturnValue(undefined);
723+
(fs.writeFileSync as jest.Mock).mockReturnValue(undefined);
724+
(fs.copyFileSync as jest.Mock).mockReturnValue(undefined);
725+
(childProcess.execFileSync as jest.Mock).mockReturnValue('');
726+
727+
const mockResolveCdsVersions = jest.mocked(
728+
jest.requireMock('../../../src/packageManager/versionResolver').resolveCdsVersions,
729+
);
730+
mockResolveCdsVersions.mockReturnValue({
731+
resolvedCdsVersion: '6.1.3',
732+
resolvedCdsDkVersion: '6.0.0',
733+
cdsExactMatch: true,
734+
cdsDkExactMatch: true,
735+
});
736+
});
737+
738+
it('should copy .npmrc to the cache directory before running npm install', () => {
739+
// Simulate an .npmrc in the project directory
740+
(fs.existsSync as jest.Mock).mockImplementation(
741+
(p: string) => p === '/source/project1/.npmrc',
742+
);
743+
744+
const dependencyGraph = createMockDependencyGraph([
745+
{
746+
projectDir: 'project1',
747+
packageJson: {
748+
name: 'project1',
749+
dependencies: { '@sap/cds': '6.1.3' },
750+
devDependencies: { '@sap/cds-dk': '6.0.0' },
751+
},
752+
},
753+
]);
754+
755+
cacheInstallDependencies(dependencyGraph, '/source', '/codeql');
756+
757+
expect(fs.copyFileSync).toHaveBeenCalledWith(
758+
'/source/project1/.npmrc',
759+
expect.stringContaining('.npmrc'),
760+
);
761+
});
762+
763+
it('should succeed even when .npmrc copy fails', () => {
764+
// Simulate an .npmrc in the project directory
765+
(fs.existsSync as jest.Mock).mockImplementation(
766+
(p: string) => p === '/source/project1/.npmrc',
767+
);
768+
(fs.copyFileSync as jest.Mock).mockImplementation(() => {
769+
throw new Error('Permission denied');
770+
});
771+
772+
const dependencyGraph = createMockDependencyGraph([
773+
{
774+
projectDir: 'project1',
775+
packageJson: {
776+
name: 'project1',
777+
dependencies: { '@sap/cds': '6.1.3' },
778+
devDependencies: { '@sap/cds-dk': '6.0.0' },
779+
},
780+
},
781+
]);
782+
783+
// Should not throw and should still attempt npm install
784+
const result = cacheInstallDependencies(dependencyGraph, '/source', '/codeql');
785+
786+
expect(childProcess.execFileSync).toHaveBeenCalledWith(
787+
'npm',
788+
['install', '--quiet', '--no-audit', '--no-fund'],
789+
expect.any(Object),
790+
);
791+
expect(result.size).toBe(1);
792+
});
793+
794+
it('should not copy .npmrc when no .npmrc file is found', () => {
795+
(fs.existsSync as jest.Mock).mockReturnValue(false);
796+
797+
const dependencyGraph = createMockDependencyGraph([
798+
{
799+
projectDir: 'project1',
800+
packageJson: {
801+
name: 'project1',
802+
dependencies: { '@sap/cds': '6.1.3' },
803+
devDependencies: { '@sap/cds-dk': '6.0.0' },
804+
},
805+
},
806+
]);
807+
808+
cacheInstallDependencies(dependencyGraph, '/source', '/codeql');
809+
810+
expect(fs.copyFileSync).not.toHaveBeenCalled();
811+
});
812+
});
641813
});

extractors/javascript/tools/pre-finalize.cmd

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ if not defined CODEQL_EXTRACTOR_CDS_SKIP_EXTRACTION (
1212
-- ^
1313
"%CODEQL_EXTRACTOR_JAVASCRIPT_WIP_DATABASE%"
1414

15+
if %ERRORLEVEL% neq 0 (
16+
echo database index-files for CDS (.cds) files failed with exit code %ERRORLEVEL%.
17+
exit /b %ERRORLEVEL%
18+
)
19+
1520
echo Finished running database index-files for CDS (.cds) files.
1621
)
1722

@@ -27,6 +32,11 @@ type NUL && "%CODEQL_DIST%\codeql.exe" database index-files ^
2732
-- ^
2833
"%CODEQL_EXTRACTOR_JAVASCRIPT_WIP_DATABASE%"
2934

35+
if %ERRORLEVEL% neq 0 (
36+
echo database index-files for UI5 (.view.xml and .fragment.xml) files failed with exit code %ERRORLEVEL%.
37+
exit /b %ERRORLEVEL%
38+
)
39+
3040
echo Finished running database index-files for UI5 (.view.xml and .fragment.xml) files.
3141

3242
REM UI5 also requires *.view.json files and *.view.html files be indexed, but these are indexed by
@@ -35,4 +45,4 @@ REM default by CodeQL.
3545
REM XSJS also requires indexing of *.xsaccess files, *.xsjs files and xs-app.json files, but these
3646
REM are indexed by default by CodeQL.
3747

38-
exit /b %ERRORLEVEL%
48+
exit /b 0

0 commit comments

Comments
 (0)