From 8c65a6540ee995116d3ec2348fcc9c005e194f81 Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Tue, 28 Nov 2023 18:29:53 +0530 Subject: [PATCH 1/3] Added custom binding support --- packages/core/src/server.js | 9 +++++-- packages/core/test/unit/server.test.js | 37 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/core/src/server.js b/packages/core/src/server.js index 7b9a42d54..f9aaa9c82 100644 --- a/packages/core/src/server.js +++ b/packages/core/src/server.js @@ -114,6 +114,11 @@ export class Server extends http.Server { }); } + // return host bind address - defaults to 0.0.0.0 + get host() { + return process.env.PERCY_SERVER_HOST || '0.0.0.0'; + } + // return the listening port or any default port get port() { return super.address()?.port ?? this.#defaultPort; @@ -122,7 +127,7 @@ export class Server extends http.Server { // return a string representation of the server address address() { let port = this.port; - let host = 'http://localhost'; + let host = `http://${this.host}`; return port ? `${host}:${port}` : host; } @@ -131,7 +136,7 @@ export class Server extends http.Server { return new Promise((resolve, reject) => { let handle = err => off() && err ? reject(err) : resolve(this); let off = () => this.off('error', handle).off('listening', handle); - super.listen(port, handle).once('error', handle); + super.listen(port, this.host, handle).once('error', handle); }); } diff --git a/packages/core/test/unit/server.test.js b/packages/core/test/unit/server.test.js index 0d49d5b0e..e2c5d1247 100644 --- a/packages/core/test/unit/server.test.js +++ b/packages/core/test/unit/server.test.js @@ -20,6 +20,43 @@ describe('Unit / Server', () => { await new Promise(r => setImmediate(setImmediate, r)); }); + describe('#host', () => { + beforeEach(async () => { + await server.listen(9000); + server.route('get', '/test/:path', (req, res) => res.text(req.params.path)); + }); + + it('returns the host', async () => { + expect(server.host).toEqual('0.0.0.0'); + + // test connection via 0.0.0.0 + await expectAsync(request('/test/foo', 'GET')).toBeResolvedTo('foo'); + + // test connection via localhost + let { request } = await import('../helpers/request.js'); + await expectAsync(request(new URL(path, 'localhost'), 'GET')).toBeResolvedTo('foo'); + }); + + describe('with PERCY_SERVER_HOST set', () => { + beforeEach(() => { + process.env.PERCY_SERVER_HOST = 'localhost'; + }); + + afterEach(() => { + delete process.env.PERCY_SERVER_HOST; + }); + + it('uses correct host', async () => { + // test connection via localhost + await expectAsync(request('/test/foo', 'GET')).toBeResolvedTo('foo'); + + // test connection via 0.0.0.0 + let { request } = await import('../helpers/request.js'); + await expectAsync(request(new URL(path, '0.0.0.0'), 'GET')).toBeRejected(); + }); + }); + }); + describe('#port', () => { it('returns the provided default port when not listening', () => { expect(server.port).toEqual(8000); From 8c99181b19b14dca501a566b99561e91c443953c Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Tue, 28 Nov 2023 19:05:54 +0530 Subject: [PATCH 2/3] Fixed tests --- packages/core/test/percy.test.js | 2 +- packages/core/test/unit/server.test.js | 45 ++++++++++++++------------ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 96000e838..3f14b81d8 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -149,7 +149,7 @@ describe('Percy', () => { describe('#address()', () => { it('returns the server API address', async () => { - expect(percy.address()).toEqual('http://localhost:5338'); + expect(percy.address()).toEqual('http://0.0.0.0:5338'); }); }); diff --git a/packages/core/test/unit/server.test.js b/packages/core/test/unit/server.test.js index e2c5d1247..eeb4ab269 100644 --- a/packages/core/test/unit/server.test.js +++ b/packages/core/test/unit/server.test.js @@ -21,20 +21,8 @@ describe('Unit / Server', () => { }); describe('#host', () => { - beforeEach(async () => { - await server.listen(9000); - server.route('get', '/test/:path', (req, res) => res.text(req.params.path)); - }); - it('returns the host', async () => { expect(server.host).toEqual('0.0.0.0'); - - // test connection via 0.0.0.0 - await expectAsync(request('/test/foo', 'GET')).toBeResolvedTo('foo'); - - // test connection via localhost - let { request } = await import('../helpers/request.js'); - await expectAsync(request(new URL(path, 'localhost'), 'GET')).toBeResolvedTo('foo'); }); describe('with PERCY_SERVER_HOST set', () => { @@ -46,13 +34,8 @@ describe('Unit / Server', () => { delete process.env.PERCY_SERVER_HOST; }); - it('uses correct host', async () => { - // test connection via localhost - await expectAsync(request('/test/foo', 'GET')).toBeResolvedTo('foo'); - - // test connection via 0.0.0.0 - let { request } = await import('../helpers/request.js'); - await expectAsync(request(new URL(path, '0.0.0.0'), 'GET')).toBeRejected(); + it('returns correct host', async () => { + expect(server.host).toEqual('localhost'); }); }); }); @@ -70,11 +53,11 @@ describe('Unit / Server', () => { describe('#address()', () => { it('returns the localhost address for the server', () => { - expect(server.address()).toEqual('http://localhost:8000'); + expect(server.address()).toEqual('http://0.0.0.0:8000'); }); it('does not include the port without a default when not listening', () => { - expect(Server.createServer().address()).toEqual('http://localhost'); + expect(Server.createServer().address()).toEqual('http://0.0.0.0'); }); }); @@ -97,6 +80,26 @@ describe('Unit / Server', () => { Server.createServer().listen(server.port) ).toBeRejected(); }); + + describe('with PERCY_SERVER_HOST set', () => { + beforeEach(() => { + process.env.PERCY_SERVER_HOST = 'localhost'; + }); + + afterEach(() => { + delete process.env.PERCY_SERVER_HOST; + }); + + it('listens on correct host', async () => { + expect(server.host).toEqual('localhost'); + await server.listen(); + server.route('get', '/test/:path', (req, res) => res.text(req.params.path)); + await expectAsync(request('/test/foo', 'GET')).toBeResolvedTo('foo'); + + // as we have a single network interface locally its not easy to test a negative test + // where with a separate network interface we are unable to access server + }); + }); }); describe('#close()', () => { From 35bcb32bc248663fbf9c640b19903641aeeb4b39 Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Wed, 29 Nov 2023 15:36:30 +0530 Subject: [PATCH 3/3] Fixed for windows --- packages/core/src/server.js | 6 ++++- packages/core/test/discovery.test.js | 11 ++++++--- packages/core/test/percy.test.js | 2 +- packages/core/test/unit/server.test.js | 31 ++++++++++++++++++++++++-- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/packages/core/src/server.js b/packages/core/src/server.js index f9aaa9c82..c51c56240 100644 --- a/packages/core/src/server.js +++ b/packages/core/src/server.js @@ -127,7 +127,11 @@ export class Server extends http.Server { // return a string representation of the server address address() { let port = this.port; - let host = `http://${this.host}`; + // we need to specifically map 0.0.0.0 to localhost on windows as even though we + // can listen to all interfaces using 0.0.0.0 we cant make a request on 0.0.0.0 as + // its an invalid ip address as per spec, but unix systems allow request to it and + // falls back to localhost + let host = `http://${this.host === '0.0.0.0' ? 'localhost' : this.host}`; return port ? `${host}:${port}` : host; } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 490b8387a..4a6a1ad2d 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1493,7 +1493,7 @@ describe('Discovery', () => { server2.reply('/img.gif', () => new Promise(resolve => { // should not resolve within the test timeout - setTimeout(resolve, 10000, [200, 'image/gif', pixel]); + setTimeout(resolve, jasmine.DEFAULT_TIMEOUT_INTERVAL + 5000, [200, 'image/gif', pixel]); })); await percy.snapshot({ @@ -1507,8 +1507,13 @@ describe('Discovery', () => { await percy.idle(); - let paths2 = server2.requests.map(r => r[0]); - expect(paths2).toContain('/img.gif'); + expect(captured[0]).not.toContain( + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://ex.localhost:8001/img.gif' + }) + }) + ); }); it('captures remote resources from allowed hostnames', async () => { diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 3f14b81d8..96000e838 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -149,7 +149,7 @@ describe('Percy', () => { describe('#address()', () => { it('returns the server API address', async () => { - expect(percy.address()).toEqual('http://0.0.0.0:5338'); + expect(percy.address()).toEqual('http://localhost:5338'); }); }); diff --git a/packages/core/test/unit/server.test.js b/packages/core/test/unit/server.test.js index eeb4ab269..39ed95621 100644 --- a/packages/core/test/unit/server.test.js +++ b/packages/core/test/unit/server.test.js @@ -53,11 +53,38 @@ describe('Unit / Server', () => { describe('#address()', () => { it('returns the localhost address for the server', () => { - expect(server.address()).toEqual('http://0.0.0.0:8000'); + // converts default 0.0.0.0 to localhost + expect(server.address()).toEqual('http://localhost:8000'); }); it('does not include the port without a default when not listening', () => { - expect(Server.createServer().address()).toEqual('http://0.0.0.0'); + expect(Server.createServer().address()).toEqual('http://localhost'); + }); + + describe('with PERCY_SERVER_HOST set', () => { + afterEach(() => { + delete process.env.PERCY_SERVER_HOST; + }); + + describe('when PERCY_SERVER_HOST=localhost', () => { + beforeEach(() => { + process.env.PERCY_SERVER_HOST = 'localhost'; + }); + + it('it uses localhost correctly', () => { + expect(Server.createServer().address()).toEqual('http://localhost'); + }); + }); + + describe('when PERCY_SERVER_HOST=120.22.12.1', () => { + beforeEach(() => { + process.env.PERCY_SERVER_HOST = '120.22.12.1'; + }); + + it('it uses 120.22.12.1 correctly', () => { + expect(Server.createServer().address()).toEqual('http://120.22.12.1'); + }); + }); }); });