Skip to content

chore: add testing documentation and coverage checks #1147

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ coverage:
status:
project:
default:
target: 80
informational: true
patch:
default:
target: 80
informational: true
4 changes: 1 addition & 3 deletions nyc.config.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const opts = {
branches: 80,
checkCoverage: true,
lines: 80,
functions: 80,
statements: 80,
};

console.log('nyc config: ', opts);
Expand Down
7 changes: 5 additions & 2 deletions src/proxy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,17 @@ export const proxyPreparations = async () => {
const defaultAuthorisedRepoList = getAuthorisedList();
const allowedList: Repo[] = await getRepos();

defaultAuthorisedRepoList.forEach(async (x) => {
await Promise.all(defaultAuthorisedRepoList.map(async (x) => {
console.log('x', x);
console.log('allowedList', allowedList);
const found = allowedList.find((y) => y.project === x.project && x.name === y.name);
console.log('found', found);
if (!found) {
await createRepo(x);
await addUserCanPush(x.name, 'admin');
await addUserCanAuthorise(x.name, 'admin');
}
});
}));
};

// just keep this async incase it needs async stuff in the future
Expand Down
145 changes: 145 additions & 0 deletions test/testProxy.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
const chai = require('chai');
const sinon = require('sinon');
const http = require('http');
const https = require('https');

const proxyquire = require('proxyquire');

const proxyModule = require('../src/proxy/index');
const expect = chai.expect;

function purgeModule(moduleName) {
try {
const resolved = require.resolve(moduleName);
const mod = require.cache[resolved];
if (!mod) return;
// recursively purge children first
mod.children.forEach((child) => {
purgeModule(child.id);
});
delete require.cache[resolved];
} catch (err) {
// ignore if module not found in cache
}
}

describe('Proxy Module', () => {
let sandbox;

beforeEach(() => {
sandbox = sinon.createSandbox();
});

afterEach(() => {
sandbox.restore();
});

describe('createApp', () => {
it('should create express app with router', async () => {
const app = await proxyModule.default.createApp();

// Basic checks for express app
expect(app).to.be.an('function');
expect(app.use).to.be.a('function');
expect(app.listen).to.be.a('function');
expect(app.settings).to.be.an('object');
});
});

describe('start', () => {
let httpCreateServerStub;
let httpsCreateServerStub;
let mockHttpServer;
let mockHttpsServer;
let getTLSEnabledStub;
let proxyPreparationsStub;

beforeEach(() => {
mockHttpServer = {
listen: sandbox.stub().callsArg(1)
};

mockHttpsServer = {
listen: sandbox.stub().callsArg(1)
};

httpCreateServerStub = sandbox.stub(http, 'createServer').returns(mockHttpServer);
httpsCreateServerStub = sandbox.stub(https, 'createServer').returns(mockHttpsServer);
getTLSEnabledStub = sandbox.stub(require('../src/config'), 'getTLSEnabled');
proxyPreparationsStub = sandbox.stub(proxyModule, 'proxyPreparations').resolves();
});

it('should start HTTP server only when TLS is disabled', async () => {
getTLSEnabledStub.returns(false);

await proxyModule.default.start();

expect(httpCreateServerStub.calledOnce).to.be.true;
expect(httpsCreateServerStub.called).to.be.false;
expect(mockHttpServer.listen.calledOnce).to.be.true;
expect(proxyPreparationsStub.calledOnce).to.be.true;
});
});

describe('proxyPreparations', () => {
let getPluginsStub
let getAuthorisedListStub
let getReposStub
let createRepoStub
let addUserCanPushStub
let addUserCanAuthoriseStub
let PluginLoaderStub

beforeEach(() => {
purgeModule('../src/proxy/index');
purgeModule('../src/config');
purgeModule('../src/db');
purgeModule('../src/plugin');
purgeModule('../src/proxy/chain');
purgeModule('../src/config/env');

getPluginsStub = sandbox.stub().returns(['fake-plugin']);
getAuthorisedListStub = sandbox.stub().returns([
{ project: 'test-proj1', name: 'repo1' },
{ project: 'test-proj2', name: 'repo2' },
]);
getReposStub = sandbox.stub().resolves([{ project: 'test-proj1', name: 'repo1' }]);
createRepoStub = sandbox.stub().resolves();
addUserCanPushStub = sandbox.stub().resolves();
addUserCanAuthoriseStub = sandbox.stub().resolves();

PluginLoaderStub = sandbox.stub().returns({ load: sandbox.stub().resolves() });
});

it('should load plugins and create missing repos', async () => {
const proxyModule = proxyquire('../src/proxy/index', {
'../config': {
getPlugins: getPluginsStub,
getAuthorisedList: getAuthorisedListStub,
getRepos: getReposStub,
getTLSEnabled: sandbox.stub().returns(false),
getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'),
getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'),
},
'../db': {
createRepo: createRepoStub,
addUserCanPush: addUserCanPushStub,
addUserCanAuthorise: addUserCanAuthoriseStub,
},
'../plugin': {
PluginLoader: PluginLoaderStub,
},
'./chain': {}
});

await proxyModule.proxyPreparations();

sinon.assert.calledOnce(PluginLoaderStub);
sinon.assert.calledWith(PluginLoaderStub, ['fake-plugin']);

expect(createRepoStub.callCount).to.equal(2);
expect(addUserCanPushStub.callCount).to.equal(2);
expect(addUserCanAuthoriseStub.callCount).to.equal(2);
});
});
});
9 changes: 7 additions & 2 deletions website/docs/development/contributing.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Here's how to get setup for contributing to GitProxy.
## Setup
The GitProxy project relies on the following pre-requisites:

- [Node](https://nodejs.org/en/download) (16+)
- [Node](https://nodejs.org/en/download) (22+)
- [npm](https://npmjs.com/) (8+)
- [git](https://git-scm.com/downloads) or equivalent Git client. It must support HTTP/S.

Expand All @@ -26,7 +26,12 @@ $ npm run client # Run only the UI
```

## Testing
<!-- Notes about testing methodology -->

Currently, we use Mocha and Chai for unit testing and Cypress for E2E testing. For more details on how to use these testing libraries, check out our [Testing documentation](testing).

### Patch coverage requirements

Newly introduced changes **must have over 80% unit test coverage**. This is enforced by our CI, and in practice, only few exceptions (such as emergency fixes) are allowed to skip this requirement. Make sure to add thorough unit tests to your PR to help reviewers approve your PR more quickly!

## Configuration schema
The configuration for GitProxy includes a JSON Schema ([`config.schema.json`](https://github.com/finos/git-proxy/blob/main/config.schema.json)) to define the expected properties used by the application. When adding new configuration properties to GitProxy, ensure that the schema is updated with any new, removed or changed properties. See [JSON Schema docs for specific syntax](https://json-schema.org/docs).
Expand Down
Loading
Loading