Merge pull request #8376 from overleaf/jpa-delete-zonal-downloads-split-test

[web] tear down split test on zonal downloads -- keep zonal variant

GitOrigin-RevId: a7e1f48cdc64175e4fc28828ad9a46b0a28383b5
This commit is contained in:
Timothée Alby
2022-06-15 08:04:18 +00:00
committed by Copybot
parent 83664d2af6
commit 0e91c44980
2 changed files with 47 additions and 131 deletions
@@ -13,7 +13,6 @@ const ClsiCookieManager = require('./ClsiCookieManager')(
Settings.apis.clsi?.backendGroupName Settings.apis.clsi?.backendGroupName
) )
const Path = require('path') const Path = require('path')
const SplitTestHandler = require('../SplitTests/SplitTestHandler')
const COMPILE_TIMEOUT_MS = 10 * 60 * 1000 const COMPILE_TIMEOUT_MS = 10 * 60 * 1000
@@ -84,38 +83,20 @@ module.exports = CompileController = {
return next(error) return next(error)
} }
Metrics.inc('compile-status', 1, { status }) Metrics.inc('compile-status', 1, { status })
SplitTestHandler.getAssignment( let pdfDownloadDomain = Settings.pdfDownloadDomain
req, if (pdfDownloadDomain && outputUrlPrefix) {
res, pdfDownloadDomain += outputUrlPrefix
'zonal-clsi-lb-downloads', }
{}, res.json({
(_err, assignment) => { status,
if (Array.isArray(outputFiles)) { outputFiles,
// NOTE: keep this around as a safeguard for rolling back clsi. compileGroup: limits?.compileGroup,
outputFiles.forEach(file => { clsiServerId,
file.url = file.url.replace(/^\/zone\/\w/, '') validationProblems,
}) stats,
} timings,
let pdfDownloadDomain = Settings.pdfDownloadDomain pdfDownloadDomain,
if ( })
assignment?.variant === 'zonal' &&
pdfDownloadDomain &&
outputUrlPrefix
) {
pdfDownloadDomain += outputUrlPrefix
}
res.json({
status,
outputFiles,
compileGroup: limits?.compileGroup,
clsiServerId,
validationProblems,
stats,
timings,
pdfDownloadDomain,
})
}
)
} }
) )
}, },
@@ -87,32 +87,36 @@ describe('CompileController', function () {
) )
}) })
describe('zonal downloads', function () { describe('pdfDownloadDomain', function () {
beforeEach(function () { beforeEach(function () {
this.settings.pdfDownloadDomain = 'https://compiles.overleaf.test' this.settings.pdfDownloadDomain = 'https://compiles.overleaf.test'
this.CompileManager.compile = sinon.stub().callsArgWith(
3,
null,
(this.status = 'success'),
(this.outputFiles = [
{
path: 'output.pdf',
url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
type: 'pdf',
},
]),
undefined, // clsiServerId
undefined, // limits
undefined, // validationProblems
undefined, // stats
undefined, // timings
'/zone/b'
)
}) })
describe('when in the default split test variant and with the old clsi deploy', function () { describe('when clsi does not emit zone prefix', function () {
beforeEach(function () {
this.CompileController.compile(this.req, this.res, this.next)
})
it('should add domain verbatim', function () {
this.res.statusCode.should.equal(200)
this.res.body.should.equal(
JSON.stringify({
status: this.status,
outputFiles: [
{
path: 'output.pdf',
url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
type: 'pdf',
},
],
pdfDownloadDomain: 'https://compiles.overleaf.test',
})
)
})
})
describe('when clsi emits a zone prefix', function () {
beforeEach(function () { beforeEach(function () {
this.getAssignment.yields(null, { variant: 'default' })
this.CompileManager.compile = sinon.stub().callsArgWith( this.CompileManager.compile = sinon.stub().callsArgWith(
3, 3,
null, null,
@@ -120,89 +124,20 @@ describe('CompileController', function () {
(this.outputFiles = [ (this.outputFiles = [
{ {
path: 'output.pdf', path: 'output.pdf',
// The previous clsi version sent the zone prefix in the url url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
url: `/zone/b/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
type: 'pdf', type: 'pdf',
}, },
]) ]),
undefined, // clsiServerId
undefined, // limits
undefined, // validationProblems
undefined, // stats
undefined, // timings
'/zone/b'
) )
this.CompileController.compile(this.req, this.res, this.next) this.CompileController.compile(this.req, this.res, this.next)
}) })
it('should remove the zone prefix', function () {
this.res.statusCode.should.equal(200)
this.res.body.should.equal(
JSON.stringify({
status: this.status,
outputFiles: [
{
path: 'output.pdf',
url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
type: 'pdf',
},
],
pdfDownloadDomain: 'https://compiles.overleaf.test',
})
)
})
})
describe('when in the default split test variant and not output files were returned', function () {
beforeEach(function () {
this.getAssignment.yields(null, { variant: 'default' })
this.CompileManager.compile = sinon
.stub()
.callsArgWith(
3,
null,
(this.status = 'success'),
(this.outputFiles = null)
)
this.CompileController.compile(this.req, this.res, this.next)
})
it('should ignore the output files', function () {
this.res.statusCode.should.equal(200)
this.res.body.should.equal(
JSON.stringify({
status: this.status,
outputFiles: null,
pdfDownloadDomain: 'https://compiles.overleaf.test',
})
)
})
})
describe('when in the default split test variant', function () {
beforeEach(function () {
this.getAssignment.yields(null, { variant: 'default' })
this.CompileController.compile(this.req, this.res, this.next)
})
it('should remove the zone prefix', function () {
this.res.statusCode.should.equal(200)
this.res.body.should.equal(
JSON.stringify({
status: this.status,
outputFiles: [
{
path: 'output.pdf',
url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
type: 'pdf',
},
],
pdfDownloadDomain: 'https://compiles.overleaf.test',
})
)
})
})
describe('when in the zonal split test variant', function () {
beforeEach(function () {
this.getAssignment.yields(null, { variant: 'zonal' })
this.CompileController.compile(this.req, this.res, this.next)
})
it('should add the zone prefix', function () { it('should add the zone prefix', function () {
this.res.statusCode.should.equal(200) this.res.statusCode.should.equal(200)
this.res.body.should.equal( this.res.body.should.equal(