From ea31b065551b580a47cc7a48e4c36c2baea1f4d1 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 10 Aug 2025 17:59:26 +0900 Subject: [PATCH 01/21] chore: add coverage check to nyc script --- nyc.config.js | 1 + 1 file changed, 1 insertion(+) diff --git a/nyc.config.js b/nyc.config.js index 7b8a33f43..785a859cf 100644 --- a/nyc.config.js +++ b/nyc.config.js @@ -1,4 +1,5 @@ const opts = { + checkCoverage: true, branches: 80, lines: 80, functions: 80, From 62be927923d84d34d57c3294cef40b2c413aae3a Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 11 Aug 2025 20:45:03 +0900 Subject: [PATCH 02/21] test: main proxy file (preliminary tests) --- test/testProxy.test.js | 73 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 test/testProxy.test.js diff --git a/test/testProxy.test.js b/test/testProxy.test.js new file mode 100644 index 000000000..356806525 --- /dev/null +++ b/test/testProxy.test.js @@ -0,0 +1,73 @@ +const chai = require('chai'); +const sinon = require('sinon'); +const express = require('express'); +const http = require('http'); +const https = require('https'); +const fs = require('fs'); + +const proxyModule = require('../src/proxy/index'); +const expect = chai.expect; + +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; + let createAppStub; + + 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(); + createAppStub = sandbox.stub(proxyModule.default, 'createApp').resolves({ express: 'app' }); + }); + + it('should start HTTP server only when TLS is disabled', async () => { + getTLSEnabledStub.returns(false); + + const app = 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; + }); + }); +}); From c76efe78d49091689759a2ea16f0cee743d9dfba Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 11 Aug 2025 20:54:50 +0900 Subject: [PATCH 03/21] chore: remove non-line coverage requirements --- nyc.config.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/nyc.config.js b/nyc.config.js index 785a859cf..4f165dfea 100644 --- a/nyc.config.js +++ b/nyc.config.js @@ -1,9 +1,6 @@ const opts = { checkCoverage: true, - branches: 80, lines: 80, - functions: 80, - statements: 80, }; console.log('nyc config: ', opts); From 2abfdec36be947029b717efedee9f85ef8d3109f Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 11 Aug 2025 22:14:31 +0900 Subject: [PATCH 04/21] test: proxyPreparations function --- test/testProxy.test.js | 76 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/test/testProxy.test.js b/test/testProxy.test.js index 356806525..14d2c6c66 100644 --- a/test/testProxy.test.js +++ b/test/testProxy.test.js @@ -3,11 +3,27 @@ const sinon = require('sinon'); const express = require('express'); const http = require('http'); const https = require('https'); -const fs = require('fs'); + +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; @@ -70,4 +86,62 @@ describe('Proxy Module', () => { expect(proxyPreparationsStub.calledOnce).to.be.true; }); }); + + describe('proxyPreparations', () => { + let getPluginsStub, getAuthorisedListStub, getReposStub; + let createRepoStub, addUserCanPushStub, 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); + }); + }); }); From 1a7b8b9a917f0e8e87f586ecabb430f3453135d8 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 11 Aug 2025 22:34:41 +0900 Subject: [PATCH 05/21] test: fix proxyPreparations repo loading and add logging --- src/proxy/index.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/proxy/index.ts b/src/proxy/index.ts index 79c91791a..aa27d3fa6 100644 --- a/src/proxy/index.ts +++ b/src/proxy/index.ts @@ -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 From f376bf6157c0c383b3a055d07c7438a181ce6991 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 12 Aug 2025 19:11:07 +0900 Subject: [PATCH 06/21] chore: cleanup and fix linter issue --- test/testProxy.test.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/testProxy.test.js b/test/testProxy.test.js index 14d2c6c66..d6f4e696f 100644 --- a/test/testProxy.test.js +++ b/test/testProxy.test.js @@ -1,6 +1,5 @@ const chai = require('chai'); const sinon = require('sinon'); -const express = require('express'); const http = require('http'); const https = require('https'); @@ -26,7 +25,7 @@ function purgeModule(moduleName) { describe('Proxy Module', () => { let sandbox; - + beforeEach(() => { sandbox = sinon.createSandbox(); }); @@ -43,7 +42,6 @@ describe('Proxy Module', () => { 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'); }); }); @@ -55,30 +53,26 @@ describe('Proxy Module', () => { let mockHttpsServer; let getTLSEnabledStub; let proxyPreparationsStub; - let createAppStub; 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(); - createAppStub = sandbox.stub(proxyModule.default, 'createApp').resolves({ express: 'app' }); }); it('should start HTTP server only when TLS is disabled', async () => { getTLSEnabledStub.returns(false); - const app = await proxyModule.default.start(); + await proxyModule.default.start(); expect(httpCreateServerStub.calledOnce).to.be.true; expect(httpsCreateServerStub.called).to.be.false; @@ -88,9 +82,13 @@ describe('Proxy Module', () => { }); describe('proxyPreparations', () => { - let getPluginsStub, getAuthorisedListStub, getReposStub; - let createRepoStub, addUserCanPushStub, addUserCanAuthoriseStub; - let PluginLoaderStub; + let getPluginsStub + let getAuthorisedListStub + let getReposStub + let createRepoStub + let addUserCanPushStub + let addUserCanAuthoriseStub + let PluginLoaderStub beforeEach(() => { purgeModule('../src/proxy/index'); From 565bd9d9561294f4b73ab3c1e884272d99538d79 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 12 Aug 2025 22:05:19 +0900 Subject: [PATCH 07/21] chore: add coverage and patch 80% targets to codecov.yml --- codecov.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codecov.yml b/codecov.yml index bfdc9877d..b7eed3138 100644 --- a/codecov.yml +++ b/codecov.yml @@ -2,7 +2,9 @@ coverage: status: project: default: + target: 80 informational: true patch: default: + target: 80 informational: true From 0240ea85041efcc71ff2a7aa7148153c9fc334e1 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:10:49 +0900 Subject: [PATCH 08/21] docs: add testing section to contributing.mdx --- website/docs/development/contributing.mdx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/website/docs/development/contributing.mdx b/website/docs/development/contributing.mdx index 4b56f7467..0ce9c4d6c 100644 --- a/website/docs/development/contributing.mdx +++ b/website/docs/development/contributing.mdx @@ -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. @@ -26,7 +26,12 @@ $ npm run client # Run only the UI ``` ## Testing - + +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). From c9829cdef046ef5f75eaac6d5e6254f51623a663 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:12:11 +0900 Subject: [PATCH 09/21] docs: add testing.mdx with unit testing section --- website/docs/development/testing.mdx | 87 ++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 website/docs/development/testing.mdx diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx new file mode 100644 index 000000000..cc3320eab --- /dev/null +++ b/website/docs/development/testing.mdx @@ -0,0 +1,87 @@ +--- +title: Testing +--- + +## Testing + +As of v1.19.2, GitProxy uses [Mocha](https://mochajs.org/) (`ts-mocha`) as the test runner, and [Chai](https://www.chaijs.com/) for unit test assertions. End-to-end (E2E) tests are written in [Cypress](https://docs.cypress.io), and some fuzz testing is done with [`fast-check`](https://fast-check.dev/). + +### Unit testing with Mocha and Chai + +Here's an example unit test that uses Chai for testing (`test/testAuthMethods.test.js`): + +```js +// Import all the test dependencies we need +const chai = require('chai'); +const sinon = require('sinon'); +const proxyquire = require('proxyquire'); + +// Import module that contains the function we want to test +const config = require('../src/config'); + +// Allows using chain-based expect calls +chai.should(); +const expect = chai.expect; + +describe('auth methods', async () => { + it('should return a local auth method by default', async function () { + const authMethods = config.getAuthMethods(); + expect(authMethods).to.have.lengthOf(1); + expect(authMethods[0].type).to.equal('local'); + }); + + it('should return an error if no auth methods are enabled', async function () { + const newConfig = JSON.stringify({ + authentication: [ + { type: 'local', enabled: false }, + { type: 'ActiveDirectory', enabled: false }, + { type: 'openidconnect', enabled: false }, + ], + }); + + const fsStub = { + existsSync: sinon.stub().returns(true), + readFileSync: sinon.stub().returns(newConfig), + }; + + const config = proxyquire('../src/config', { + fs: fsStub, + }); + + // Initialize the user config after proxyquiring to load the stubbed config + config.initUserConfig(); + + expect(() => config.getAuthMethods()).to.throw(Error, 'No authentication method enabled'); + }); + + it('should return an array of enabled auth methods when overridden', async function () { + const newConfig = JSON.stringify({ + authentication: [ + { type: 'local', enabled: true }, + { type: 'ActiveDirectory', enabled: true }, + { type: 'openidconnect', enabled: true }, + ], + }); + + const fsStub = { + existsSync: sinon.stub().returns(true), + readFileSync: sinon.stub().returns(newConfig), + }; + + const config = proxyquire('../src/config', { + fs: fsStub, + }); + + // Initialize the user config after proxyquiring to load the stubbed config + config.initUserConfig(); + + const authMethods = config.getAuthMethods(); + expect(authMethods).to.have.lengthOf(3); + expect(authMethods[0].type).to.equal('local'); + expect(authMethods[1].type).to.equal('ActiveDirectory'); + expect(authMethods[2].type).to.equal('openidconnect'); + }); +}); +``` + +Core concepts to keep in mind when unit testing JS/TS modules with Chai: From 52f8da181b105641a72c40114f272984c29a911f Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:13:42 +0900 Subject: [PATCH 10/21] docs: add more details to unit test section --- website/docs/development/testing.mdx | 99 ++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index cc3320eab..696f0049f 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -85,3 +85,102 @@ describe('auth methods', async () => { ``` Core concepts to keep in mind when unit testing JS/TS modules with Chai: + +#### Stub internal methods to make tests predictable + +Functions often make use of internal libraries such as `fs` for reading files and performing operations that are dependent on the overall state of the app (or database/filesystem). Since we're only testing that the given function behaves the way we want, we **stub** these libraries. + +For example, here we stub the `fs` library so that "reading" the `proxy.config.json` file returns our mock config file: + +```js +// Define the mock config file +const newConfig = JSON.stringify({ + authentication: [ + { type: 'local', enabled: true }, + { type: 'ActiveDirectory', enabled: true }, + { type: 'openidconnect', enabled: true }, + ], +}); + +// Create the stub for `fs.existsSync` and `fs.readFileSync` +const fsStub = { + existsSync: sinon.stub().returns(true), + readFileSync: sinon.stub().returns(newConfig), +}; +``` + +This stub will make all calls to `fs.existsSync` to return `true` and all calls to `readFileSync` to return the `newConfig` mock file. + +Then, we use `proxyquire` to plug in the stub to the library that we're testing: + +```js +const config = proxyquire('../src/config', { + fs: fsStub, +}); + +// Initialize the user config after proxyquiring to load the stubbed config +config.initUserConfig(); +``` + +Finally, when calling the function we're trying to test, the internal calls will automatically resolve to the values we chose. + +#### Setup and cleanup + +`before` and `beforeEach`, `after` and `afterEach` are testing constructs that allow executing code before and after each test. This allows setting up stubs before each test, making API calls, setting up the database - or otherwise cleaning up the database after test execution. + +This is an example from another test file (`test/addRepoTest.test.js`): + +```js +before(async function () { + app = await service.start(); + + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); + await db.createUser('u1', 'abc', 'test@test.com', 'test', true); + await db.createUser('u2', 'abc', 'test2@test.com', 'test', true); +}); + +// Tests go here + +after(async function () { + await service.httpServer.close(); + + await db.deleteRepo('test-repo'); + await db.deleteUser('u1'); + await db.deleteUser('u2'); +}); +``` + +#### Focus on expected behaviour + +Mocha and Chai make it easy to write tests in plain English. It's a good idea to write the expected behaviour in plain English and then prove it by writing the test: + +```js +describe('auth methods', async () => { + it('should return a local auth method by default', async function () { + // Test goes here + }); + + it('should return an error if no auth methods are enabled', async function () { + // Test goes here + }); + + it('should return an array of enabled auth methods when overridden', async function () { + // Test goes here + }); +}); +``` + +Assertions can also be done similarly to plain English: + +```js +expect(authMethods).to.have.lengthOf(3); +expect(authMethods[0].type).to.equal('local'); +``` + +#### Unit testing coverage requirement + +**All new lines of code introduced in a PR, must have over 80% coverage** (patch coverage). This is enforced by our CI, and generally a PR will not be merged unless this coverage requirement is met. Please make sure to write thorough unit tests to increase GitProxy's code quality! + +If test coverage is still insufficient after writing your tests, check out the [CodeCov report](https://app.codecov.io/gh/finos/git-proxy) after making the PR and take a look at which lines are missing coverage. From ad97cd52766201c1776457ac6dc6b82682095c2c Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:15:10 +0900 Subject: [PATCH 11/21] docs: add e2e testing with cypress section --- website/docs/development/testing.mdx | 91 ++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index 696f0049f..5813a888e 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -184,3 +184,94 @@ expect(authMethods[0].type).to.equal('local'); **All new lines of code introduced in a PR, must have over 80% coverage** (patch coverage). This is enforced by our CI, and generally a PR will not be merged unless this coverage requirement is met. Please make sure to write thorough unit tests to increase GitProxy's code quality! If test coverage is still insufficient after writing your tests, check out the [CodeCov report](https://app.codecov.io/gh/finos/git-proxy) after making the PR and take a look at which lines are missing coverage. + +### E2E testing with Cypress + +Although coverage is currently low, we have introduced Cypress testing to make sure that end-to-end flows are working as expected with every added feature. + +This is a sample test from `cypress/e2e/repo.cy.js`: + +```js +describe('Repo', () => { + beforeEach(() => { + // Custom login command + cy.login('admin', 'admin'); + + cy.visit('/dashboard/repo'); + + // prevent failures on 404 request and uncaught promises + cy.on('uncaught:exception', () => false); + }); + + describe('Code button for repo row', () => { + it('Opens tooltip with correct content and can copy', () => { + const cloneURL = 'http://localhost:8000/finos/git-proxy.git'; + const tooltipQuery = 'div[role="tooltip"]'; + + cy + // tooltip isn't open to start with + .get(tooltipQuery) + .should('not.exist'); + + cy + // find the entry for finos/git-proxy + .get('a[href="/dashboard/repo/git-proxy"]') + // take it's parent row + .closest('tr') + // find the nearby span containing Code we can click to open the tooltip + .find('span') + .contains('Code') + .should('exist') + .click(); + + cy + // find the newly opened tooltip + .get(tooltipQuery) + .should('exist') + .find('span') + // check it contains the url we expect + .contains(cloneURL) + .should('exist') + .parent() + // find the adjacent span that contains the svg + .find('span') + .next() + // check it has the copy icon first and click it + .get('svg.octicon-copy') + .should('exist') + .click() + // check the icon has changed to the check icon + .get('svg.octicon-copy') + .should('not.exist') + .get('svg.octicon-check') + .should('exist'); + + // failed to successfully check the clipboard + }); + }); +}); +``` + +Here, we use a similar syntax to Mocha to **describe the behaviour that we expect**. The difference, is that Cypress expects us to write actual commands for executing actions in the app. Some commands used very often include `visit` (navigates to a certain page), `get` (gets a certain page element to check its properties), `contains` (checks if an element has a certain string value in it), `should` (similar to `expect` in unit tests). + +#### Custom commands + +Cypress allows defining **custom commands** to reuse and simplify code. + +In the above example, `cy.login('admin', 'admin')` is actually a custom command defined in `/cypress/support/commands.js`. It allows logging a user into the app, which is a requirement for many E2E flows: + +```js +Cypress.Commands.add('login', (username, password) => { + cy.session([username, password], () => { + cy.visit('/login'); + cy.intercept('GET', '**/api/auth/me').as('getUser'); + + cy.get('[data-test=username]').type(username); + cy.get('[data-test=password]').type(password); + cy.get('[data-test=login]').click(); + + cy.wait('@getUser'); + cy.url().should('include', '/dashboard/repo'); + }); +}); +``` From f52ef08b1179e734fe0d4b19956d1eb213096d57 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:15:35 +0900 Subject: [PATCH 12/21] docs: add fuzz testing section --- website/docs/development/testing.mdx | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index 5813a888e..f7bdd06fc 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -275,3 +275,39 @@ Cypress.Commands.add('login', (username, password) => { }); }); ``` + +### Fuzz testing with fast-check + +Fuzz testing helps find edge case bugs by generating random inputs for test data. This is very helpful since regular tests often have naive assumptions of users always inputting "expected" data. + +Fuzz testing with fast-check is very easy: it integrates seamlessly with Mocha and it doesn't require any additional libraries beyond fast-check itself. + +Here's an example of a fuzz test section for a test file (`testCheckRepoInAuthList.test.js`): + +```js +const fc = require('fast-check'); + +// Unit tests go here + +describe('fuzzing', () => { + it('should not crash on random repo names', async () => { + await fc.assert( + fc.asyncProperty( + fc.string(), + async (repoName) => { + const action = new actions.Action('123', 'type', 'get', 1234, repoName); + const result = await processor.exec(null, action, authList); + expect(result.error).to.be.true; + } + ), + { numRuns: 100 } + ); + }); +}); +``` + +Writing fuzz tests is a bit different from regular unit tests, although we do still `assert` whether a certain value is correct or not. In this example, fc.string() indicates that a random string value is being generated for the `repoName` variable. This `repoName` is then inserted in the `action` to see if the `processor.exec()` function is capable of handling these or not. + +In this case, we expect that the `result.error` value is always true. This means that the `exec` flow always errors out, but never crashes the app entirely. You may also want to test that the app is always able to complete a flow without an error. + +Finally, we have the `numRuns` property for `fc.assert`. This allows us to run the fuzz test multiple times with a new randomized value each time. This is important since the test may randomly fail or pass depending on the input. From 16f78b713392af692e289def90757a4265047899 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Wed, 13 Aug 2025 19:36:06 +0900 Subject: [PATCH 13/21] docs: add development/testing to links --- website/sidebars.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/sidebars.js b/website/sidebars.js index 6573101d1..c778eca85 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -43,7 +43,7 @@ module.exports = { }, collapsible: true, collapsed: false, - items: ['development/contributing', 'development/plugins'], + items: ['development/contributing', 'development/plugins', 'development/testing'], }, ], }; From a8380604fdf7c9a8d6f853dc1688226177d2d2ff Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sun, 17 Aug 2025 21:49:48 +0900 Subject: [PATCH 14/21] chore: remove logging --- src/proxy/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/proxy/index.ts b/src/proxy/index.ts index aa27d3fa6..6a7a43808 100644 --- a/src/proxy/index.ts +++ b/src/proxy/index.ts @@ -44,10 +44,8 @@ export const proxyPreparations = async () => { const allowedList: Repo[] = await getRepos(); 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'); From 73c5de04e9cdd6136ae3c22dba853428d90771bc Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 21 Aug 2025 19:04:38 +0900 Subject: [PATCH 15/21] test: refactor proxy tests --- test/testProxy.test.js | 363 ++++++++++++++++++++++++++++++----------- 1 file changed, 264 insertions(+), 99 deletions(-) diff --git a/test/testProxy.test.js b/test/testProxy.test.js index d6f4e696f..d311fbf94 100644 --- a/test/testProxy.test.js +++ b/test/testProxy.test.js @@ -2,144 +2,309 @@ 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', () => { +describe('Proxy', () => { let sandbox; + let Proxy; + let mockHttpServer; + let mockHttpsServer; beforeEach(() => { sandbox = sinon.createSandbox(); + + mockHttpServer = { + listen: sandbox.stub().callsFake((port, callback) => { + if (callback) setImmediate(callback); + return mockHttpServer; + }), + close: sandbox.stub().callsFake((callback) => { + if (callback) setImmediate(callback); + return mockHttpServer; + }) + }; + + mockHttpsServer = { + listen: sandbox.stub().callsFake((port, callback) => { + if (callback) setImmediate(callback); + return mockHttpsServer; + }), + close: sandbox.stub().callsFake((callback) => { + if (callback) setImmediate(callback); + return mockHttpsServer; + }) + }; + + sandbox.stub(http, 'createServer').returns(mockHttpServer); + sandbox.stub(https, 'createServer').returns(mockHttpsServer); + + // deep mocking for express router + const mockRouter = sandbox.stub(); + mockRouter.use = sandbox.stub(); + mockRouter.get = sandbox.stub(); + mockRouter.post = sandbox.stub(); + mockRouter.stack = []; + + Proxy = proxyquire('../src/proxy/index', { + './routes': { + getRouter: sandbox.stub().resolves(mockRouter) + }, + '../config': { + getTLSEnabled: sandbox.stub().returns(false), + getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), + getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), + getPlugins: sandbox.stub().returns(['mock-plugin']), + getAuthorisedList: sandbox.stub().returns([ + { project: 'test-proj', name: 'test-repo' } + ]) + }, + '../db': { + getRepos: sandbox.stub().resolves([]), + createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), + addUserCanPush: sandbox.stub().resolves(), + addUserCanAuthorise: sandbox.stub().resolves() + }, + '../plugin': { + PluginLoader: sandbox.stub().returns({ + load: sandbox.stub().resolves() + }) + }, + './chain': { + default: {} + }, + '../config/env': { + serverConfig: { + GIT_PROXY_SERVER_PORT: 3000, + GIT_PROXY_HTTPS_SERVER_PORT: 3001 + } + }, + 'fs': { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) + } + }).default; }); afterEach(() => { sandbox.restore(); }); - describe('createApp', () => { - it('should create express app with router', async () => { - const app = await proxyModule.default.createApp(); + describe('start()', () => { + it('should start HTTP server when TLS is disabled', async () => { + const proxy = new Proxy(); - // 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'); + await proxy.start(); + + expect(http.createServer.calledOnce).to.be.true; + expect(https.createServer.called).to.be.false; + expect(mockHttpServer.listen.calledWith(3000)).to.be.true; + + await proxy.stop(); + }); + + it('should start both HTTP and HTTPS servers when TLS is enabled', async () => { + const mockRouterTLS = sandbox.stub(); + mockRouterTLS.use = sandbox.stub(); + mockRouterTLS.get = sandbox.stub(); + mockRouterTLS.post = sandbox.stub(); + mockRouterTLS.stack = []; + + const ProxyWithTLS = proxyquire('../src/proxy/index', { + './routes': { + getRouter: sandbox.stub().resolves(mockRouterTLS) + }, + '../config': { + getTLSEnabled: sandbox.stub().returns(true), // TLS enabled + getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), + getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), + getPlugins: sandbox.stub().returns(['mock-plugin']), + getAuthorisedList: sandbox.stub().returns([]) + }, + '../db': { + getRepos: sandbox.stub().resolves([]), + createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), + addUserCanPush: sandbox.stub().resolves(), + addUserCanAuthorise: sandbox.stub().resolves() + }, + '../plugin': { + PluginLoader: sandbox.stub().returns({ + load: sandbox.stub().resolves() + }) + }, + './chain': { + default: {} + }, + '../config/env': { + serverConfig: { + GIT_PROXY_SERVER_PORT: 3000, + GIT_PROXY_HTTPS_SERVER_PORT: 3001 + } + }, + 'fs': { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) + } + }).default; + + const proxy = new ProxyWithTLS(); + + await proxy.start(); + + expect(http.createServer.calledOnce).to.be.true; + expect(https.createServer.calledOnce).to.be.true; + expect(mockHttpServer.listen.calledWith(3000)).to.be.true; + expect(mockHttpsServer.listen.calledWith(3001)).to.be.true; + + await proxy.stop(); + }); + + it('should set up express app after starting', async () => { + const proxy = new Proxy(); + expect(proxy.getExpressApp()).to.be.null; + + await proxy.start(); + + expect(proxy.getExpressApp()).to.not.be.null; + expect(proxy.getExpressApp()).to.be.a('function'); + + await proxy.stop(); }); }); - 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(); + describe('getExpressApp()', () => { + it('should return null before start() is called', () => { + const proxy = new Proxy(); + + expect(proxy.getExpressApp()).to.be.null; }); - it('should start HTTP server only when TLS is disabled', async () => { - getTLSEnabledStub.returns(false); + it('should return express app after start() is called', async () => { + const proxy = new Proxy(); - await proxyModule.default.start(); + await proxy.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; + const app = proxy.getExpressApp(); + expect(app).to.not.be.null; + expect(app).to.be.a('function'); + expect(app.use).to.be.a('function'); + + await proxy.stop(); }); }); - 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() }); + describe('stop()', () => { + it('should close HTTP server when running', async () => { + const proxy = new Proxy(); + await proxy.start(); + await proxy.stop(); + + expect(mockHttpServer.close.calledOnce).to.be.true; }); - it('should load plugins and create missing repos', async () => { - const proxyModule = proxyquire('../src/proxy/index', { + it('should close both HTTP and HTTPS servers when both are running', async () => { + const mockRouterStop = sandbox.stub(); + mockRouterStop.use = sandbox.stub(); + mockRouterStop.get = sandbox.stub(); + mockRouterStop.post = sandbox.stub(); + mockRouterStop.stack = []; + + const ProxyWithTLS = proxyquire('../src/proxy/index', { + './routes': { + getRouter: sandbox.stub().resolves(mockRouterStop) + }, '../config': { - getPlugins: getPluginsStub, - getAuthorisedList: getAuthorisedListStub, - getRepos: getReposStub, - getTLSEnabled: sandbox.stub().returns(false), + getTLSEnabled: sandbox.stub().returns(true), getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), + getPlugins: sandbox.stub().returns([]), + getAuthorisedList: sandbox.stub().returns([]) }, '../db': { - createRepo: createRepoStub, - addUserCanPush: addUserCanPushStub, - addUserCanAuthorise: addUserCanAuthoriseStub, + getRepos: sandbox.stub().resolves([]), + createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), + addUserCanPush: sandbox.stub().resolves(), + addUserCanAuthorise: sandbox.stub().resolves() }, '../plugin': { - PluginLoader: PluginLoaderStub, + PluginLoader: sandbox.stub().returns({ + load: sandbox.stub().resolves() + }) + }, + './chain': { + default: {} + }, + '../config/env': { + serverConfig: { + GIT_PROXY_SERVER_PORT: 3000, + GIT_PROXY_HTTPS_SERVER_PORT: 3001 + } }, - './chain': {} + 'fs': { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) + } + }).default; + + const proxy = new ProxyWithTLS(); + await proxy.start(); + await proxy.stop(); + + expect(mockHttpServer.close.calledOnce).to.be.true; + expect(mockHttpsServer.close.calledOnce).to.be.true; + }); + + it('should resolve successfully when no servers are running', async () => { + const proxy = new Proxy(); + + await proxy.stop(); + + expect(mockHttpServer.close.called).to.be.false; + expect(mockHttpsServer.close.called).to.be.false; + }); + + it('should handle errors gracefully', async () => { + const proxy = new Proxy(); + await proxy.start(); + + // simulate error in server close + mockHttpServer.close.callsFake((callback) => { + throw new Error('Server close error'); }); - await proxyModule.proxyPreparations(); + try { + await proxy.stop(); + expect.fail('Expected stop() to reject'); + } catch (error) { + expect(error.message).to.equal('Server close error'); + } + }); + }); - sinon.assert.calledOnce(PluginLoaderStub); - sinon.assert.calledWith(PluginLoaderStub, ['fake-plugin']); + describe('full lifecycle', () => { + it('should start and stop successfully', async () => { + const proxy = new Proxy(); - expect(createRepoStub.callCount).to.equal(2); - expect(addUserCanPushStub.callCount).to.equal(2); - expect(addUserCanAuthoriseStub.callCount).to.equal(2); + await proxy.start(); + expect(proxy.getExpressApp()).to.not.be.null; + expect(mockHttpServer.listen.calledOnce).to.be.true; + + await proxy.stop(); + expect(mockHttpServer.close.calledOnce).to.be.true; + }); + + it('should handle multiple start/stop cycles', async () => { + const proxy = new Proxy(); + + await proxy.start(); + await proxy.stop(); + + mockHttpServer.listen.resetHistory(); + mockHttpServer.close.resetHistory(); + + await proxy.start(); + await proxy.stop(); + + expect(mockHttpServer.listen.calledOnce).to.be.true; + expect(mockHttpServer.close.calledOnce).to.be.true; }); }); }); From 96eabcd98cfdbb909d3e17344e202345ddc875b7 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Thu, 21 Aug 2025 19:51:21 +0900 Subject: [PATCH 16/21] chore: add node v20 suggestions --- package.json | 3 +++ website/docs/development/contributing.mdx | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 48ba92b92..8728f143b 100644 --- a/package.json +++ b/package.json @@ -142,5 +142,8 @@ "last 1 firefox version", "last 1 safari version" ] + }, + "engines": { + "node": ">=20.19.2" } } diff --git a/website/docs/development/contributing.mdx b/website/docs/development/contributing.mdx index 0ce9c4d6c..a8bb44c61 100644 --- a/website/docs/development/contributing.mdx +++ b/website/docs/development/contributing.mdx @@ -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) (22+) +- [Node](https://nodejs.org/en/download) (20+) - [npm](https://npmjs.com/) (8+) - [git](https://git-scm.com/downloads) or equivalent Git client. It must support HTTP/S. From 0a1ed18898b93ef446255519c4a2cf310d680653 Mon Sep 17 00:00:00 2001 From: Juan Escalada <97265671+jescalada@users.noreply.github.com> Date: Fri, 22 Aug 2025 02:24:20 +0000 Subject: [PATCH 17/21] Update website/docs/development/testing.mdx Co-authored-by: Kris West Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com> --- website/docs/development/testing.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index f7bdd06fc..2f08f342d 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -4,7 +4,7 @@ title: Testing ## Testing -As of v1.19.2, GitProxy uses [Mocha](https://mochajs.org/) (`ts-mocha`) as the test runner, and [Chai](https://www.chaijs.com/) for unit test assertions. End-to-end (E2E) tests are written in [Cypress](https://docs.cypress.io), and some fuzz testing is done with [`fast-check`](https://fast-check.dev/). +As of v1.19.2, GitProxy uses [Mocha](https://mochajs.org/) (`ts-mocha`) as the test runner, and [Chai](https://www.chaijs.com/) for unit test assertions. User interface tests are written in [Cypress](https://docs.cypress.io), and some fuzz testing is done with [`fast-check`](https://fast-check.dev/). ### Unit testing with Mocha and Chai From 6f411136fc156fd24b8ac0f0213244e0570462b0 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 22 Aug 2025 11:59:16 +0900 Subject: [PATCH 18/21] docs: add cache reset details to testing.mdx --- README.md | 4 ++-- website/docs/development/contributing.mdx | 4 ++-- website/docs/development/testing.mdx | 29 ++++++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b40a146c6..86184c523 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,6 @@ If you can't access Slack, you can also [subscribe to our mailing list](mailto:g Otherwise, if you have a deeper query or require more support, please [raise an issue](https://github.com/finos/git-proxy/issues) ๐Ÿงต -๐Ÿค Join our [fortnightly Zoom meeting](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595) on Monday, 4PM BST (odd week numbers). -๐ŸŒ [Convert to your local time](https://www.timeanddate.com/worldclock) +๐Ÿค Join our [fortnightly Zoom meeting](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595) on Monday, 4PM BST (odd week numbers). +๐ŸŒ [Convert to your local time](https://www.timeanddate.com/worldclock) ๐Ÿ“… [Click here](https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MTRvbzM0NG01dWNvNGc4OGJjNWphM2ZtaTZfMjAyNTA2MDJUMTUwMDAwWiBzYW0uaG9sbWVzQGNvbnRyb2wtcGxhbmUuaW8&tmsrc=sam.holmes%40control-plane.io&scp=ALL) for the recurring Google Calendar meeting invite. Alternatively, send an e-mail to [help@finos.org](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595#:~:text=Need-,an,-invite%3F) to get a calendar invitation. diff --git a/website/docs/development/contributing.mdx b/website/docs/development/contributing.mdx index a8bb44c61..54ba6fdc7 100644 --- a/website/docs/development/contributing.mdx +++ b/website/docs/development/contributing.mdx @@ -48,8 +48,8 @@ When updating the configuration schema, you must also re-generate the reference ## Community Meetings -Join our [fortnightly Zoom meeting](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595) on Monday, 4PM BST (odd week numbers). -๐ŸŒ [Convert to your local time](https://www.timeanddate.com/worldclock) +Join our [fortnightly Zoom meeting](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595) on Monday, 4PM BST (odd week numbers). +๐ŸŒ [Convert to your local time](https://www.timeanddate.com/worldclock) [Click here](https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MTRvbzM0NG01dWNvNGc4OGJjNWphM2ZtaTZfMjAyNTA2MDJUMTUwMDAwWiBzYW0uaG9sbWVzQGNvbnRyb2wtcGxhbmUuaW8&tmsrc=sam.holmes%40control-plane.io&scp=ALL) for the recurring Google Calendar meeting invite. Alternatively, send an e-mail to [help@finos.org](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595#:~:text=Need-,an,-invite%3F) to get a calendar invitation. diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index 2f08f342d..c4de57c45 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -150,6 +150,33 @@ after(async function () { await db.deleteUser('u1'); await db.deleteUser('u2'); }); + +afterEach(() => { + sinon.restore(); +}); +``` + +Note that `after` will execute once after **all** the tests are complete, whereas `afterEach` will execute at the end of **each** test. + +#### Reset sinon and proxyquire cache + +**It's very important to reset Sinon and the Proxyquire/require cache after each test** when necessary. This prevents old stubs from leaking into subsequent tests. + +Here is an example of a function that resets both of these after each test (`test/chain.test.js`): + +```js +const clearCache = (sandbox) => { + delete require.cache[require.resolve('../src/proxy/processors')]; + delete require.cache[require.resolve('../src/proxy/chain')]; + sandbox.reset(); +}; + +... + +afterEach(() => { + // Clear the module from the cache after each test + clearCache(sandboxSinon); +}); ``` #### Focus on expected behaviour @@ -185,7 +212,7 @@ expect(authMethods[0].type).to.equal('local'); If test coverage is still insufficient after writing your tests, check out the [CodeCov report](https://app.codecov.io/gh/finos/git-proxy) after making the PR and take a look at which lines are missing coverage. -### E2E testing with Cypress +### UI testing with Cypress Although coverage is currently low, we have introduced Cypress testing to make sure that end-to-end flows are working as expected with every added feature. From 73e862ebe62421cf938650ebfea03b2098374c3e Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 30 Aug 2025 14:29:06 +0900 Subject: [PATCH 19/21] docs: update 1.test.js with stubbing and proxyquire examples --- README.md | 2 + test/1.test.js | 88 +++++++++++++++++++++++++--- website/docs/development/testing.mdx | 6 +- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 86184c523..0909a252b 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,8 @@ customize for your environment, see the [project's documentation](https://git-pr - [Quickstart](https://git-proxy.finos.org/docs/category/quickstart/) - [Installation](https://git-proxy.finos.org/docs/installation) - [Configuration](https://git-proxy.finos.org/docs/category/configuration) +- [Contributing](https://git-proxy.finos.org/docs/development/contributing) +- [Testing](https://git-proxy.finos.org/docs/development/testing) ## Contributing diff --git a/test/1.test.js b/test/1.test.js index 227dc0104..7d6cc1dda 100644 --- a/test/1.test.js +++ b/test/1.test.js @@ -1,24 +1,98 @@ -// This test needs to run first +/* + Template test file. Demonstrates how to: + - Use chai-http to test the server + - Initialize the server + - Stub dependencies with sinon sandbox + - Reset stubs after each test + - Use proxyquire to replace modules + - Clear module cache after a test +*/ + const chai = require('chai'); const chaiHttp = require('chai-http'); +const sinon = require('sinon'); +const proxyquire = require('proxyquire'); + const service = require('../src/service'); +const db = require('../src/db'); +const { readFileSync } = require('fs'); + +const expect = chai.expect; chai.use(chaiHttp); -chai.should(); -// Use this test as a template -describe('init', async () => { +const TEST_REPO = { + project: 'finos', + name: 'db-test-repo', + url: 'https://github.com/finos/db-test-repo.git', +}; + +describe('init', () => { let app; + let sandbox; + + // Runs before all tests before(async function () { - app = await service.start(); // pass in proxy if testing config loading or administration of proxy routes + // Start the service (can also pass config if testing proxy routes) + app = await service.start(); + }); + + // Runs before each test + beforeEach(function () { + // Create a sandbox for stubbing + sandbox = sinon.createSandbox(); + + // Example: stub a DB method + sandbox.stub(db, 'getRepo').resolves(TEST_REPO); }); - it('should not be logged in', async function () { + // Example test: check server is running + it('should return 401 if not logged in', async function () { const res = await chai.request(app).get('/api/auth/profile'); + expect(res).to.have.status(401); + }); + + // Example test: check db stub is working + it('should get the repo from stubbed db', async function () { + const repo = await db.getRepo('finos/db-test-repo'); + expect(repo).to.deep.equal(TEST_REPO); + }); + + // Example test: use proxyquire to override the config module + it('should return an array of enabled auth methods when overridden', async function () { + const fsStub = { + readFileSync: sandbox.stub().returns( + JSON.stringify({ + authentication: [ + { type: 'local', enabled: true }, + { type: 'ActiveDirectory', enabled: true }, + { type: 'openidconnect', enabled: true }, + ], + }) + ), + }; + + const config = proxyquire('../src/config', { + fs: fsStub, + }); + config.initUserConfig(); + const authMethods = config.getAuthMethods(); + expect(authMethods).to.have.lengthOf(3); + expect(authMethods[0].type).to.equal('local'); + expect(authMethods[1].type).to.equal('ActiveDirectory'); + expect(authMethods[2].type).to.equal('openidconnect'); + + // Clear config module cache so other tests don't use the stubbed config + delete require.cache[require.resolve('../src/config')]; + }); - res.should.have.status(401); + // Runs after each test + afterEach(function () { + // Restore all stubs in this sandbox + sandbox.restore(); }); + // Runs after all tests after(async function () { await service.httpServer.close(); }); diff --git a/website/docs/development/testing.mdx b/website/docs/development/testing.mdx index c4de57c45..81c20b007 100644 --- a/website/docs/development/testing.mdx +++ b/website/docs/development/testing.mdx @@ -168,7 +168,7 @@ Here is an example of a function that resets both of these after each test (`tes const clearCache = (sandbox) => { delete require.cache[require.resolve('../src/proxy/processors')]; delete require.cache[require.resolve('../src/proxy/chain')]; - sandbox.reset(); + sandbox.restore(); }; ... @@ -212,6 +212,10 @@ expect(authMethods[0].type).to.equal('local'); If test coverage is still insufficient after writing your tests, check out the [CodeCov report](https://app.codecov.io/gh/finos/git-proxy) after making the PR and take a look at which lines are missing coverage. +#### More examples + +Check out [test/1.test.js](https://github.com/finos/git-proxy/blob/main/test/1.test.js) for another example on how to write unit tests. + ### UI testing with Cypress Although coverage is currently low, we have introduced Cypress testing to make sure that end-to-end flows are working as expected with every added feature. From 8566f2a0a03a91a415ca51d43685ba820cb6f8ea Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 30 Aug 2025 14:44:32 +0900 Subject: [PATCH 20/21] chore: lint --- test/1.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/1.test.js b/test/1.test.js index 7d6cc1dda..edb6a01fb 100644 --- a/test/1.test.js +++ b/test/1.test.js @@ -15,7 +15,6 @@ const proxyquire = require('proxyquire'); const service = require('../src/service'); const db = require('../src/db'); -const { readFileSync } = require('fs'); const expect = chai.expect; @@ -68,7 +67,7 @@ describe('init', () => { { type: 'ActiveDirectory', enabled: true }, { type: 'openidconnect', enabled: true }, ], - }) + }), ), }; From 8d2c69bee769260ac471c85e637f67f88de69d4b Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Sat, 30 Aug 2025 14:54:38 +0900 Subject: [PATCH 21/21] chore: format --- README.md | 2 +- test/testProxy.test.js | 74 ++++++++++++++++++++---------------------- 2 files changed, 37 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 0909a252b..93dd7fbbc 100644 --- a/README.md +++ b/README.md @@ -120,4 +120,4 @@ Otherwise, if you have a deeper query or require more support, please [raise an ๐Ÿค Join our [fortnightly Zoom meeting](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595) on Monday, 4PM BST (odd week numbers). ๐ŸŒ [Convert to your local time](https://www.timeanddate.com/worldclock) -๐Ÿ“… [Click here](https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MTRvbzM0NG01dWNvNGc4OGJjNWphM2ZtaTZfMjAyNTA2MDJUMTUwMDAwWiBzYW0uaG9sbWVzQGNvbnRyb2wtcGxhbmUuaW8&tmsrc=sam.holmes%40control-plane.io&scp=ALL) for the recurring Google Calendar meeting invite. Alternatively, send an e-mail to [help@finos.org](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595#:~:text=Need-,an,-invite%3F) to get a calendar invitation. +๐Ÿ“… [Click here](https://calendar.google.com/calendar/event?action=TEMPLATE&tmeid=MTRvbzM0NG01dWNvNGc4OGJjNWphM2ZtaTZfMjAyNTA2MDJUMTUwMDAwWiBzYW0uaG9sbWVzQGNvbnRyb2wtcGxhbmUuaW8&tmsrc=sam.holmes%40control-plane.io&scp=ALL) for the recurring Google Calendar meeting invite. Alternatively, send an e-mail to [help@finos.org](https://zoom-lfx.platform.linuxfoundation.org/meeting/95849833904?password=99413314-d03a-4b1c-b682-1ede2c399595#:~:text=Need-,an,-invite%3F) to get a calendar invitation. diff --git a/test/testProxy.test.js b/test/testProxy.test.js index d311fbf94..99508b982 100644 --- a/test/testProxy.test.js +++ b/test/testProxy.test.js @@ -23,7 +23,7 @@ describe('Proxy', () => { close: sandbox.stub().callsFake((callback) => { if (callback) setImmediate(callback); return mockHttpServer; - }) + }), }; mockHttpsServer = { @@ -34,7 +34,7 @@ describe('Proxy', () => { close: sandbox.stub().callsFake((callback) => { if (callback) setImmediate(callback); return mockHttpsServer; - }) + }), }; sandbox.stub(http, 'createServer').returns(mockHttpServer); @@ -49,40 +49,38 @@ describe('Proxy', () => { Proxy = proxyquire('../src/proxy/index', { './routes': { - getRouter: sandbox.stub().resolves(mockRouter) + getRouter: sandbox.stub().resolves(mockRouter), }, '../config': { getTLSEnabled: sandbox.stub().returns(false), getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), getPlugins: sandbox.stub().returns(['mock-plugin']), - getAuthorisedList: sandbox.stub().returns([ - { project: 'test-proj', name: 'test-repo' } - ]) + getAuthorisedList: sandbox.stub().returns([{ project: 'test-proj', name: 'test-repo' }]), }, '../db': { getRepos: sandbox.stub().resolves([]), createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), addUserCanPush: sandbox.stub().resolves(), - addUserCanAuthorise: sandbox.stub().resolves() + addUserCanAuthorise: sandbox.stub().resolves(), }, '../plugin': { PluginLoader: sandbox.stub().returns({ - load: sandbox.stub().resolves() - }) + load: sandbox.stub().resolves(), + }), }, './chain': { - default: {} + default: {}, }, '../config/env': { serverConfig: { GIT_PROXY_SERVER_PORT: 3000, - GIT_PROXY_HTTPS_SERVER_PORT: 3001 - } + GIT_PROXY_HTTPS_SERVER_PORT: 3001, + }, + }, + fs: { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')), }, - 'fs': { - readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) - } }).default; }); @@ -112,38 +110,38 @@ describe('Proxy', () => { const ProxyWithTLS = proxyquire('../src/proxy/index', { './routes': { - getRouter: sandbox.stub().resolves(mockRouterTLS) + getRouter: sandbox.stub().resolves(mockRouterTLS), }, '../config': { getTLSEnabled: sandbox.stub().returns(true), // TLS enabled getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), getPlugins: sandbox.stub().returns(['mock-plugin']), - getAuthorisedList: sandbox.stub().returns([]) + getAuthorisedList: sandbox.stub().returns([]), }, '../db': { getRepos: sandbox.stub().resolves([]), createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), addUserCanPush: sandbox.stub().resolves(), - addUserCanAuthorise: sandbox.stub().resolves() + addUserCanAuthorise: sandbox.stub().resolves(), }, '../plugin': { PluginLoader: sandbox.stub().returns({ - load: sandbox.stub().resolves() - }) + load: sandbox.stub().resolves(), + }), }, './chain': { - default: {} + default: {}, }, '../config/env': { serverConfig: { GIT_PROXY_SERVER_PORT: 3000, - GIT_PROXY_HTTPS_SERVER_PORT: 3001 - } + GIT_PROXY_HTTPS_SERVER_PORT: 3001, + }, + }, + fs: { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')), }, - 'fs': { - readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) - } }).default; const proxy = new ProxyWithTLS(); @@ -210,38 +208,38 @@ describe('Proxy', () => { const ProxyWithTLS = proxyquire('../src/proxy/index', { './routes': { - getRouter: sandbox.stub().resolves(mockRouterStop) + getRouter: sandbox.stub().resolves(mockRouterStop), }, '../config': { getTLSEnabled: sandbox.stub().returns(true), getTLSKeyPemPath: sandbox.stub().returns('/tmp/key.pem'), getTLSCertPemPath: sandbox.stub().returns('/tmp/cert.pem'), getPlugins: sandbox.stub().returns([]), - getAuthorisedList: sandbox.stub().returns([]) + getAuthorisedList: sandbox.stub().returns([]), }, '../db': { getRepos: sandbox.stub().resolves([]), createRepo: sandbox.stub().resolves({ _id: 'mock-repo-id' }), addUserCanPush: sandbox.stub().resolves(), - addUserCanAuthorise: sandbox.stub().resolves() + addUserCanAuthorise: sandbox.stub().resolves(), }, '../plugin': { PluginLoader: sandbox.stub().returns({ - load: sandbox.stub().resolves() - }) + load: sandbox.stub().resolves(), + }), }, './chain': { - default: {} + default: {}, }, '../config/env': { serverConfig: { GIT_PROXY_SERVER_PORT: 3000, - GIT_PROXY_HTTPS_SERVER_PORT: 3001 - } + GIT_PROXY_HTTPS_SERVER_PORT: 3001, + }, + }, + fs: { + readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')), }, - 'fs': { - readFileSync: sandbox.stub().returns(Buffer.from('mock-cert')) - } }).default; const proxy = new ProxyWithTLS(); @@ -302,7 +300,7 @@ describe('Proxy', () => { await proxy.start(); await proxy.stop(); - + expect(mockHttpServer.listen.calledOnce).to.be.true; expect(mockHttpServer.close.calledOnce).to.be.true; });