From 89e349146dc175bd1a32fe65d6832b2198f51de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 18:02:06 +0200 Subject: [PATCH] Fix tests and stop using sinon as a dev dependency --- package-lock.json | 154 ---------------- package.json | 1 - test/integration/pubsub-metrics-test.js | 206 ++++++++++------------ test/support/test-client.js | 7 +- test/unit/backends/pubsub-metrics-test.js | 40 ----- 5 files changed, 102 insertions(+), 306 deletions(-) delete mode 100644 test/unit/backends/pubsub-metrics-test.js diff --git a/package-lock.json b/package-lock.json index 80c09e41..b776d336 100644 --- a/package-lock.json +++ b/package-lock.json @@ -331,64 +331,6 @@ "resolved": "https://registry.npmjs.org/@protobufjs/utf8/-/utf8-1.1.0.tgz", "integrity": "sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA=" }, - "@sinonjs/commons": { - "version": "1.7.1", - "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.7.1.tgz", - "integrity": "sha512-Debi3Baff1Qu1Unc3mjJ96MgpbwTn43S1+9yJ0llWygPwDNu2aaWBD6yc9y/Z8XDRNhx7U+u2UDg2OGQXkclUQ==", - "dev": true, - "requires": { - "type-detect": "4.0.8" - } - }, - "@sinonjs/fake-timers": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-6.0.0.tgz", - "integrity": "sha512-atR1J/jRXvQAb47gfzSK8zavXy7BcpnYq21ALon0U99etu99vsir0trzIO3wpeLtW+LLVY6X7EkfVTbjGSH8Ww==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0" - } - }, - "@sinonjs/formatio": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-5.0.0.tgz", - "integrity": "sha512-ejFRrFNMaTAmhg9u1lYKJQxDocowta6KQKFnBE7XtZb/AAPlLkWQQSaqwlGYnDWQ6paXzyM1vbMhLAujSFiVPw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1", - "@sinonjs/samsam": "^4.2.0" - }, - "dependencies": { - "@sinonjs/samsam": { - "version": "4.2.2", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-4.2.2.tgz", - "integrity": "sha512-z9o4LZUzSD9Hl22zV38aXNykgFeVj8acqfFabCY6FY83n/6s/XwNJyYYldz6/9lBJanpno9h+oL6HTISkviweA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - } - } - }, - "@sinonjs/samsam": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-5.0.1.tgz", - "integrity": "sha512-iSZdE68szyFvV8ReYve6t4gAA1rLVwGyyhWBg9qrz8VAn1FH141gdg0NJcMrAJ069rD2XM2KQzY8ZNDgmTfBQA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - }, - "@sinonjs/text-encoding": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz", - "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==", - "dev": true - }, "@types/duplexify": { "version": "3.6.0", "resolved": "https://registry.npmjs.org/@types/duplexify/-/duplexify-3.6.0.tgz", @@ -4078,12 +4020,6 @@ "verror": "1.10.0" } }, - "just-extend": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz", - "integrity": "sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw==", - "dev": true - }, "jwa": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.0.tgz", @@ -4175,12 +4111,6 @@ "integrity": "sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI=", "dev": true }, - "lodash.get": { - "version": "4.4.2", - "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", - "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", - "dev": true - }, "lodash.has": { "version": "4.5.2", "resolved": "https://registry.npmjs.org/lodash.has/-/lodash.has-4.5.2.tgz", @@ -4549,52 +4479,6 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, - "nise": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/nise/-/nise-4.0.1.tgz", - "integrity": "sha512-10PKL272rqg80o2RsWcTT6X9cDYqJ4kXqPTf8yCXPc9hbphZSDmbiG5FqUNeR5nouKCQMM24ld45kgYnBdx2rw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0", - "@sinonjs/fake-timers": "^6.0.0", - "@sinonjs/formatio": "^4.0.1", - "@sinonjs/text-encoding": "^0.7.1", - "just-extend": "^4.0.2", - "path-to-regexp": "^1.7.0" - }, - "dependencies": { - "@sinonjs/formatio": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-4.0.1.tgz", - "integrity": "sha512-asIdlLFrla/WZybhm0C8eEzaDNNrzymiTqHMeJl6zPW2881l3uuVRpm0QlRQEjqYWv6CcKMGYME3LbrLJsORBw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1", - "@sinonjs/samsam": "^4.2.0" - } - }, - "@sinonjs/samsam": { - "version": "4.2.2", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-4.2.2.tgz", - "integrity": "sha512-z9o4LZUzSD9Hl22zV38aXNykgFeVj8acqfFabCY6FY83n/6s/XwNJyYYldz6/9lBJanpno9h+oL6HTISkviweA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - }, - "path-to-regexp": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz", - "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==", - "dev": true, - "requires": { - "isarray": "0.0.1" - } - } - } - }, "nock": { "version": "9.2.6", "resolved": "https://registry.npmjs.org/nock/-/nock-9.2.6.tgz", @@ -5811,44 +5695,6 @@ "resolved": "https://registry.npmjs.org/simple-statistics/-/simple-statistics-0.9.2.tgz", "integrity": "sha1-PjXLEDCPx2ljqk7nJS5qbqENKOQ=" }, - "sinon": { - "version": "9.0.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.0.0.tgz", - "integrity": "sha512-c4bREcvuK5VuEGyMW/Oim9I3Rq49Vzb0aMdxouFaA44QCFpilc5LJOugrX+mkrvikbqCimxuK+4cnHVNnLR41g==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0", - "@sinonjs/fake-timers": "^6.0.0", - "@sinonjs/formatio": "^5.0.0", - "@sinonjs/samsam": "^5.0.1", - "diff": "^4.0.2", - "nise": "^4.0.1", - "supports-color": "^7.1.0" - }, - "dependencies": { - "diff": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", - "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", - "dev": true - }, - "has-flag": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", - "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", - "dev": true - }, - "supports-color": { - "version": "7.1.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.1.0.tgz", - "integrity": "sha512-oRSIpR8pxT1Wr2FquTNnGet79b3BWljqOuoW/h4oBhxJ/HUbX5nX6JSruTkvXDCFMwDPvsaTTbvMLKZWSy0R5g==", - "dev": true, - "requires": { - "has-flag": "^4.0.0" - } - } - } - }, "slice-ansi": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-2.1.0.tgz", diff --git a/package.json b/package.json index 3ee5f8c1..f062d445 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "nock": "9.2.6", "nyc": "^14.1.1", "redis": "2.8.0", - "sinon": "^9.0.0", "step": "1.0.0", "strftime": "0.10.0" }, diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index 8e215f91..aea595c0 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -1,166 +1,156 @@ 'use strict'; -const sinon = require('sinon'); const assert = require('assert'); -const redis = require('redis'); const TestClient = require('../support/test-client'); const PubSubMetricsBackend = require('../../lib/backends/pubsub-metrics'); -const metricsHeaders = { - 'Carto-Event': 'test-event', - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': '1' -}; - -const tooLongField = ' If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean ' + - 'this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; - -const badHeaders = { - 'Carto-Event': tooLongField, - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': 1 -}; - const mapConfig = { - version: '1.7.0', + version: '1.8.0', layers: [ { options: { - sql: 'select * FROM test_table_localhost_regular1', + sql: TestClient.SQL.ONE_POINT, cartocss: TestClient.CARTOCSS.POINTS, cartocss_version: '2.3.0' } } ] }; +const apikey = 1234; -function buildEventAttributes (statusCode) { - return { - event_source: 'test', - user_id: '1', - event_group_id: '1', - response_code: statusCode.toString(), - source_domain: 'localhost', - event_time: new Date().toISOString(), - event_version: '1' - }; -} - -const fakeTopic = { - name: 'test-topic', - publish: sinon.stub().returns(Promise.resolve()) -}; - -const fakePubSub = { - topic: () => fakeTopic -}; - -describe.skip('pubsub metrics middleware', function () { - let redisClient; - let testClient; - let clock; - - before(function () { - redisClient = redis.createClient(global.environment.redis.port); - clock = sinon.useFakeTimers(); - sinon.stub(PubSubMetricsBackend, 'createPubSub').returns(fakePubSub); +describe('pubsub metrics middleware', function () { + beforeEach(function () { + this.originalPubSubMetricsBackendSendMethod = PubSubMetricsBackend.prototype.send; + this.pubSubMetricsBackendSendMethodCalled = false; + PubSubMetricsBackend.prototype.send = (event, attributes) => { + this.pubSubMetricsBackendSendMethodCalled = true; + this.pubSubMetricsBackendSendMethodCalledWith = { event, attributes }; + return Promise.resolve(); + }; }); - after(function () { - clock.restore(); - PubSubMetricsBackend.createPubSub.restore(); - global.environment.pubSubMetrics.enabled = false; - }); - - afterEach(function (done) { - fakeTopic.publish.resetHistory(); - - redisClient.SELECT(0, () => { - redisClient.del('user:localhost:mapviews:global'); - - redisClient.SELECT(5, () => { - redisClient.del('user:localhost:mapviews:global'); - done(); - }); - }); + afterEach(function () { + PubSubMetricsBackend.prototype.send = this.originalPubSubMetricsBackendSendMethod; }); it('should not send event if not enabled', function (done) { - global.environment.pubSubMetrics.enabled = false; - testClient = new TestClient(mapConfig, 1234, metricsHeaders); + const extraHeaders = { + 'Carto-Event': 'test-event', + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': '1' + }; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.notCalled); - return done(); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); + + return testClient.drain(done); }); }); it('should not send event if headers not present', function (done) { - global.environment.pubSubMetrics.enabled = true; - testClient = new TestClient(mapConfig, 1234); + const extraHeaders = {}; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.notCalled); - return done(); - }); - }); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - it('should normalized headers type and length', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(200); - const maxLength = 100; - const eventName = tooLongField.trim().substr(0, maxLength); - - testClient = new TestClient(mapConfig, 1234, badHeaders); - - testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.calledOnceWith(Buffer.from(eventName), eventAttributes)); - return done(); + return testClient.drain(done); }); }); it('should send event for map requests', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(200); - testClient = new TestClient(mapConfig, 1234, metricsHeaders); + const expectedEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - return done(); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); + }); + }); + + it('should normalized headers type and length', function (done) { + const eventLong = 'If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; + const expectedEvent = eventLong.trim().substr(0, 100); + const expectedEventGroupId = '1'; + const expectedEventSource = 'test'; + const extraHeaders = { + 'Carto-Event': eventLong, + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': 1 + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); }); }); it('should send event when error', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(400); - eventAttributes.user_id = undefined; + const expectedEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const emptyMapConfig = {}; + const testClient = new TestClient(emptyMapConfig, apikey, extraHeaders, overrideServerOptions); + const params = { response: { status: 400 } }; - testClient = new TestClient({}, 1234, metricsHeaders); + testClient.getLayergroup(params, (err, body) => { + if (err) { + return done(err); + } - testClient.getLayergroup(() => { - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - assert(fakeTopic.publish.calledOnce); - return done(); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); }); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index f0798dad..cdb59eaf 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -23,13 +23,14 @@ const MAPNIK_SUPPORTED_FORMATS = { mvt: true }; -function TestClient (config, apiKey, extraHeaders) { +function TestClient (config, apiKey, extraHeaders = {}, overrideServerOptions = {}) { this.mapConfig = isMapConfig(config) ? config : null; this.template = isTemplate(config) ? config : null; this.apiKey = apiKey; - this.extraHeaders = extraHeaders || {}; + this.extraHeaders = extraHeaders; this.keysToDelete = {}; - this.server = new CartodbWindshaft(serverOptions); + this.serverOptions = Object.assign({}, serverOptions, overrideServerOptions); + this.server = new CartodbWindshaft(this.serverOptions); } module.exports = TestClient; diff --git a/test/unit/backends/pubsub-metrics-test.js b/test/unit/backends/pubsub-metrics-test.js deleted file mode 100644 index 3660718b..00000000 --- a/test/unit/backends/pubsub-metrics-test.js +++ /dev/null @@ -1,40 +0,0 @@ -'use strict'; - -const sinon = require('sinon'); -const assert = require('assert'); -const PubSubMetricsBackend = require('../../../lib/backends/pubsub-metrics'); - -const fakeTopic = { - name: 'test-topic', - publish: sinon.stub().returns(Promise.resolve()) -}; - -const fakePubSub = { - topic: () => fakeTopic -}; - -const eventAttributes = { - event_source: 'test', - user_id: '123', - event_group_id: '1', - response_code: '200', - source_domain: 'localhost', - event_time: new Date().toISOString(), - event_version: '1' -}; - -describe.skip('pubsub metrics backend', function () { - it('should not send event if not enabled', function () { - const pubSubMetricsService = new PubSubMetricsBackend(fakePubSub, false); - - pubSubMetricsService.sendEvent('test-event', eventAttributes); - assert(fakeTopic.publish.notCalled); - }); - - it('should send event if enabled', function () { - const pubSubMetricsService = new PubSubMetricsBackend(fakePubSub, true); - - pubSubMetricsService.sendEvent('test-event', eventAttributes); - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - }); -});