From 2a3c2dd3d5b94cfa56d9189326f613efac1d66a5 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 7 May 2020 18:57:35 +0200 Subject: [PATCH 1/2] [misc] simplify the smoke test and process shutdown --- services/clsi/app.js | 61 ++++--------- services/clsi/package-lock.json | 100 ---------------------- services/clsi/package.json | 1 - services/clsi/test/smoke/js/SmokeTests.js | 92 ++++++++++---------- 4 files changed, 65 insertions(+), 189 deletions(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index 6fd29b0adb..311fc31e6a 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -17,7 +17,7 @@ if ((Settings.sentry != null ? Settings.sentry.dsn : undefined) != null) { logger.initializeErrorReporting(Settings.sentry.dsn) } -const smokeTest = require('smoke-test-sharelatex') +const smokeTest = require('./test/smoke/js/SmokeTests') const ContentTypeMapper = require('./app/js/ContentTypeMapper') const Errors = require('./app/js/Errors') @@ -192,64 +192,39 @@ app.get('/oops', function(req, res, next) { app.get('/status', (req, res, next) => res.send('CLSI is alive\n')) -const resCacher = { - contentType(setContentType) { - this.setContentType = setContentType - }, - send(code, body) { - this.code = code - this.body = body - }, - - // default the server to be down - code: 500, - body: {}, - setContentType: 'application/json' -} - -let shutdownTime +let PROCESS_IS_TOO_OLD = false if (Settings.processLifespanLimitMs) { Settings.processLifespanLimitMs += Settings.processLifespanLimitMs * (Math.random() / 10) - shutdownTime = Date.now() + Settings.processLifespanLimitMs - logger.info('Lifespan limited to ', shutdownTime) -} + logger.info( + 'Lifespan limited to ', + Date.now() + Settings.processLifespanLimitMs + ) -const checkIfProcessIsTooOld = function(cont) { - if (shutdownTime && shutdownTime < Date.now()) { + setTimeout(() => { logger.log('shutting down, process is too old') - resCacher.send = function() {} - resCacher.code = 500 - resCacher.body = { processToOld: true } - } else { - cont() - } + PROCESS_IS_TOO_OLD = true + }, Settings.processLifespanLimitMs) } if (Settings.smokeTest) { - const runSmokeTest = function() { - checkIfProcessIsTooOld(function() { - logger.log('running smoke tests') - smokeTest.run( - require.resolve(__dirname + '/test/smoke/js/SmokeTests.js') - )({}, resCacher) - return setTimeout(runSmokeTest, 30 * 1000) + function runSmokeTest() { + if (PROCESS_IS_TOO_OLD) return + logger.log('running smoke tests') + smokeTest.triggerRun(err => { + logger.error({ err }, 'smoke tests failed') + setTimeout(runSmokeTest, 30 * 1000) }) } runSmokeTest() } app.get('/health_check', function(req, res) { - res.contentType(resCacher.setContentType) - return res.status(resCacher.code).send(resCacher.body) + if (PROCESS_IS_TOO_OLD) return res.status(500).json({ processToOld: true }) + smokeTest.sendLastResult(res) }) -app.get('/smoke_test_force', (req, res) => - smokeTest.run(require.resolve(__dirname + '/test/smoke/js/SmokeTests.js'))( - req, - res - ) -) +app.get('/smoke_test_force', (req, res) => smokeTest.sendNewResult(res)) app.use(function(error, req, res, next) { if (error instanceof Errors.NotFoundError) { diff --git a/services/clsi/package-lock.json b/services/clsi/package-lock.json index 2d27ff12ba..7f896457cd 100644 --- a/services/clsi/package-lock.json +++ b/services/clsi/package-lock.json @@ -1669,11 +1669,6 @@ "delayed-stream": "~1.0.0" } }, - "commander": { - "version": "2.0.0", - "resolved": "http://registry.npmjs.org/commander/-/commander-2.0.0.tgz", - "integrity": "sha1-0bhvkB+LZL2UG96tr5JFMDk76Sg=" - }, "common-tags": { "version": "1.8.0", "resolved": "https://registry.npmjs.org/common-tags/-/common-tags-1.8.0.tgz", @@ -1878,11 +1873,6 @@ "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-1.0.3.tgz", "integrity": "sha1-+hN8S9aY7fVc1c0CrFWfkaTEups=" }, - "diff": { - "version": "1.0.7", - "resolved": "https://registry.npmjs.org/diff/-/diff-1.0.7.tgz", - "integrity": "sha1-JLuwAcSn1VIhaefKvbLCgU7ZHPQ=" - }, "diskusage": { "version": "1.1.3", "resolved": "https://registry.npmjs.org/diskusage/-/diskusage-1.1.3.tgz", @@ -3066,11 +3056,6 @@ "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.3.tgz", "integrity": "sha512-a30VEBm4PEdx1dRB7MFK7BejejvCvBronbLjht+sHuGYj8PHs7M/5Z+rt5lw551vZ7yfTCj4Vuyy3mSJytDWRQ==" }, - "growl": { - "version": "1.7.0", - "resolved": "https://registry.npmjs.org/growl/-/growl-1.7.0.tgz", - "integrity": "sha1-3i1mE20ALhErpw8/EMMc98NQsto=" - }, "gtoken": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/gtoken/-/gtoken-4.1.0.tgz", @@ -3576,27 +3561,6 @@ "resolved": "https://registry.npmjs.org/isstream/-/isstream-0.1.2.tgz", "integrity": "sha1-R+Y/evVa+m+S4VAOaQ64uFKcCZo=" }, - "jade": { - "version": "0.26.3", - "resolved": "https://registry.npmjs.org/jade/-/jade-0.26.3.tgz", - "integrity": "sha1-jxDXl32NefL2/4YqgbBRPMslaGw=", - "requires": { - "commander": "0.6.1", - "mkdirp": "0.3.0" - }, - "dependencies": { - "commander": { - "version": "0.6.1", - "resolved": "https://registry.npmjs.org/commander/-/commander-0.6.1.tgz", - "integrity": "sha1-+mihT2qUXVTbvlDYzbMyDp47GgY=" - }, - "mkdirp": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.0.tgz", - "integrity": "sha1-G79asbqCevI1dRQ0kEJkVfSB/h4=" - } - } - }, "js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -6062,11 +6026,6 @@ "object-inspect": "^1.7.0" } }, - "sigmund": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/sigmund/-/sigmund-1.0.1.tgz", - "integrity": "sha1-P/IfGYytIXX587eBhT/ZTQ0ZtZA=" - }, "signal-exit": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.2.tgz", @@ -6129,65 +6088,6 @@ } } }, - "smoke-test-sharelatex": { - "version": "git+https://github.com/sharelatex/smoke-test-sharelatex.git#bc3e93d18ccee219c0d99e8b02c984ccdd842e1c", - "from": "git+https://github.com/sharelatex/smoke-test-sharelatex.git#v0.2.0", - "requires": { - "mocha": "~1.17.0" - }, - "dependencies": { - "glob": { - "version": "3.2.3", - "resolved": "https://registry.npmjs.org/glob/-/glob-3.2.3.tgz", - "integrity": "sha1-4xPusknHr/qlxHUoaw4RW1mDlGc=", - "requires": { - "graceful-fs": "~2.0.0", - "inherits": "2", - "minimatch": "~0.2.11" - } - }, - "graceful-fs": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-2.0.3.tgz", - "integrity": "sha1-fNLNsiiko/Nule+mzBQt59GhNtA=" - }, - "lru-cache": { - "version": "2.7.3", - "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-2.7.3.tgz", - "integrity": "sha1-bUUk6LlV+V1PW1iFHOId1y+06VI=" - }, - "minimatch": { - "version": "0.2.14", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-0.2.14.tgz", - "integrity": "sha1-x054BXT2PG+aCQ6Q775u9TpqdWo=", - "requires": { - "lru-cache": "2", - "sigmund": "~1.0.0" - } - }, - "mocha": { - "version": "1.17.1", - "resolved": "https://registry.npmjs.org/mocha/-/mocha-1.17.1.tgz", - "integrity": "sha1-f3Zx1oUm0HS3uuZgyQmfh+DqHMs=", - "requires": { - "commander": "2.0.0", - "debug": "*", - "diff": "1.0.7", - "glob": "3.2.3", - "growl": "1.7.x", - "jade": "0.26.3", - "mkdirp": "0.3.5" - }, - "dependencies": { - "mkdirp": { - "version": "0.3.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.5.tgz", - "integrity": "sha1-3j5fiWHIjHh+4TaN+EmsRBPsqNc=" - } - } - } - } - }, "snakecase-keys": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/snakecase-keys/-/snakecase-keys-3.1.0.tgz", diff --git a/services/clsi/package.json b/services/clsi/package.json index 5c25fa1207..e57bfa5518 100644 --- a/services/clsi/package.json +++ b/services/clsi/package.json @@ -34,7 +34,6 @@ "request": "^2.88.2", "sequelize": "^5.21.5", "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#v1.1.0", - "smoke-test-sharelatex": "git+https://github.com/sharelatex/smoke-test-sharelatex.git#v0.2.0", "sqlite3": "^4.1.1", "underscore": "^1.9.2", "v8-profiler-node8": "^6.1.1", diff --git a/services/clsi/test/smoke/js/SmokeTests.js b/services/clsi/test/smoke/js/SmokeTests.js index 851ea85079..fc52121ee0 100644 --- a/services/clsi/test/smoke/js/SmokeTests.js +++ b/services/clsi/test/smoke/js/SmokeTests.js @@ -1,20 +1,3 @@ -/* eslint-disable - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -const chai = require('chai') -if (Object.prototype.should == null) { - chai.should() -} -const { expect } = chai const request = require('request') const Settings = require('settings-sharelatex') @@ -23,9 +6,35 @@ const buildUrl = path => const url = buildUrl(`project/smoketest-${process.pid}/compile`) -describe('Running a compile', function() { - before(function(done) { - return request.post( +module.exports = { + sendNewResult(res) { + this._run(error => this._sendResponse(res, error)) + }, + sendLastResult(res) { + this._sendResponse(res, this._lastError) + }, + triggerRun(cb) { + this._run(error => { + this._lastError = error + cb(error) + }) + }, + + _lastError: new Error('SmokeTestsPending'), + _sendResponse(res, error) { + let code, body + if (error) { + code = 500 + body = error.message + } else { + code = 200 + body = 'OK' + } + res.contentType('text/plain') + res.status(code).send(body) + }, + _run(done) { + request.post( { url, json: { @@ -50,7 +59,7 @@ describe('Running a compile', function() { \\pgfmathsetmacro{\\dy}{rand*0.1}% A random variance in the y coordinate, % gives a hight fill to the lipid \\pgfmathsetmacro{\\rot}{rand*0.1}% A random variance in the - % molecule orientation + % molecule orientation \\shade[ball color=red] ({\\i+\\dx+\\rot},{0.5*\\j+\\dy+0.4*sin(\\i*\\nuPi*10)}) circle(0.45); \\shade[ball color=gray] (\\i+\\dx,{0.5*\\j+\\dy+0.4*sin(\\i*\\nuPi*10)-0.9}) circle(0.45); \\shade[ball color=gray] (\\i+\\dx-\\rot,{0.5*\\j+\\dy+0.4*sin(\\i*\\nuPi*10)-1.8}) circle(0.45); @@ -72,29 +81,22 @@ describe('Running a compile', function() { } }, (error, response, body) => { - this.error = error - this.response = response - this.body = body - return done() + if (error) return done(error) + if (!body || !body.compile || !body.compile.outputFiles) { + return done(new Error('response payload incomplete')) + } + + let pdfFound = false + let logFound = false + for (const file of body.compile.outputFiles) { + if (file.type === 'pdf') pdfFound = true + if (file.type === 'log') logFound = true + } + + if (!pdfFound) return done(new Error('no pdf returned')) + if (!logFound) return done(new Error('no log returned')) + done() } ) - }) - - it('should return the pdf', function() { - for (const file of Array.from(this.body.compile.outputFiles)) { - if (file.type === 'pdf') { - return - } - } - throw new Error('no pdf returned') - }) - - return it('should return the log', function() { - for (const file of Array.from(this.body.compile.outputFiles)) { - if (file.type === 'log') { - return - } - } - throw new Error('no log returned') - }) -}) + } +} From 36e81cbe15a3599a6386c630ea950981d273e071 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 11 May 2020 13:08:13 +0200 Subject: [PATCH 2/2] [misc] apply review feedback --- services/clsi/app.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index 311fc31e6a..add5f21720 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -192,7 +192,7 @@ app.get('/oops', function(req, res, next) { app.get('/status', (req, res, next) => res.send('CLSI is alive\n')) -let PROCESS_IS_TOO_OLD = false +Settings.processTooOld = false if (Settings.processLifespanLimitMs) { Settings.processLifespanLimitMs += Settings.processLifespanLimitMs * (Math.random() / 10) @@ -203,16 +203,16 @@ if (Settings.processLifespanLimitMs) { setTimeout(() => { logger.log('shutting down, process is too old') - PROCESS_IS_TOO_OLD = true + Settings.processTooOld = true }, Settings.processLifespanLimitMs) } if (Settings.smokeTest) { function runSmokeTest() { - if (PROCESS_IS_TOO_OLD) return + if (Settings.processTooOld) return logger.log('running smoke tests') smokeTest.triggerRun(err => { - logger.error({ err }, 'smoke tests failed') + if (err) logger.error({ err }, 'smoke tests failed') setTimeout(runSmokeTest, 30 * 1000) }) } @@ -220,7 +220,9 @@ if (Settings.smokeTest) { } app.get('/health_check', function(req, res) { - if (PROCESS_IS_TOO_OLD) return res.status(500).json({ processToOld: true }) + if (Settings.processTooOld) { + return res.status(500).json({ processTooOld: true }) + } smokeTest.sendLastResult(res) })