Skip to content

Commit ac3bb18

Browse files
ryanthemanuelryanthemanuel
authored
internal: (studio) enable studio to access the protocol database with a new connection (#31502)
* internal: (studio) improvements to how studio can access the protocol database * fix * fix build * refactor due to downstream changes * Update packages/app/src/studio/studio-app-types.ts * Update packages/server/lib/cloud/studio.ts
1 parent 480d7d9 commit ac3bb18

File tree

11 files changed

+143
-59
lines changed

11 files changed

+143
-59
lines changed

‎.circleci/workflows.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ mainBuildFilters: &mainBuildFilters
3838
- /^release\/\d+\.\d+\.\d+$/
3939
# use the following branch as well to ensure that v8 snapshot cache updates are fully tested
4040
- 'update-v8-snapshot-cache-on-develop'
41-
- 'chore/update_mobx_decoratorless'
41+
- 'ryanm/chore/full-snapshot-threaded'
4242

4343
# usually we don't build Mac app - it takes a long time
4444
# but sometimes we want to really confirm we are doing the right thing
@@ -49,7 +49,7 @@ macWorkflowFilters: &darwin-workflow-filters
4949
- equal: [ develop, << pipeline.git.branch >> ]
5050
# use the following branch as well to ensure that v8 snapshot cache updates are fully tested
5151
- equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
52-
- equal: [ 'chore/update_mobx_decoratorless', << pipeline.git.branch >> ]
52+
- equal: [ 'ryanm/chore/full-snapshot-threaded', << pipeline.git.branch >> ]
5353
- matches:
5454
pattern: /^release\/\d+\.\d+\.\d+$/
5555
value: << pipeline.git.branch >>
@@ -60,7 +60,7 @@ linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
6060
- equal: [ develop, << pipeline.git.branch >> ]
6161
# use the following branch as well to ensure that v8 snapshot cache updates are fully tested
6262
- equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
63-
- equal: [ 'chore/update_mobx_decoratorless', << pipeline.git.branch >> ]
63+
- equal: [ 'ryanm/chore/full-snapshot-threaded', << pipeline.git.branch >> ]
6464
- matches:
6565
pattern: /^release\/\d+\.\d+\.\d+$/
6666
value: << pipeline.git.branch >>
@@ -83,7 +83,7 @@ windowsWorkflowFilters: &windows-workflow-filters
8383
- equal: [ develop, << pipeline.git.branch >> ]
8484
# use the following branch as well to ensure that v8 snapshot cache updates are fully tested
8585
- equal: [ 'update-v8-snapshot-cache-on-develop', << pipeline.git.branch >> ]
86-
- equal: [ 'chore/update_mobx_decoratorless', << pipeline.git.branch >> ]
86+
- equal: [ 'ryanm/chore/full-snapshot-threaded', << pipeline.git.branch >> ]
8787
- matches:
8888
pattern: /^release\/\d+\.\d+\.\d+$/
8989
value: << pipeline.git.branch >>
@@ -157,7 +157,7 @@ commands:
157157
name: Set environment variable to determine whether or not to persist artifacts
158158
command: |
159159
echo "Setting SHOULD_PERSIST_ARTIFACTS variable"
160-
echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "chore/update_mobx_decoratorless" ]]; then
160+
echo 'if ! [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "ryanm/chore/full-snapshot-threaded" ]]; then
161161
export SHOULD_PERSIST_ARTIFACTS=true
162162
fi' >> "$BASH_ENV"
163163
# You must run `setup_should_persist_artifacts` command and be using bash before running this command

‎packages/server/lib/cloud/protocol.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ export class ProtocolManager implements ProtocolManagerShape {
6262
} : undefined
6363
}
6464

65-
get db () {
66-
return this._db
65+
get dbPath () {
66+
return this._dbPath
6767
}
6868

6969
async prepareProtocol (script: string, options: ProtocolManagerOptions) {

‎packages/server/lib/cloud/studio.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { StudioErrorReport, StudioManagerShape, StudioStatus, StudioServerDefaultShape, StudioServerShape, ProtocolManagerShape, StudioCloudApi } from '@packages/types'
1+
import type { StudioErrorReport, StudioManagerShape, StudioStatus, StudioServerDefaultShape, StudioServerShape, ProtocolManagerShape, StudioCloudApi, StudioAIInitializeOptions } from '@packages/types'
22
import type { Router } from 'express'
33
import type { Socket } from 'socket.io'
44
import fetch from 'cross-fetch'
@@ -7,7 +7,7 @@ import os from 'os'
77
import { agent } from '@packages/network'
88
import Debug from 'debug'
99
import { requireScript } from './require_script'
10-
import type Database from 'better-sqlite3'
10+
import path from 'path'
1111

1212
interface StudioServer { default: StudioServerDefaultShape }
1313

@@ -39,17 +39,14 @@ export class StudioManager implements StudioManagerShape {
3939
return manager
4040
}
4141

42-
setProtocolDb (db: Database.Database): void {
43-
this.invokeSync('setProtocolDb', { isEssential: true }, db)
44-
}
45-
4642
async setup ({ script, studioPath, studioHash, projectSlug, cloudApi }: SetupOptions): Promise<void> {
4743
const { createStudioServer } = requireScript<StudioServer>(script).default
4844

4945
this._studioServer = await createStudioServer({
5046
studioPath,
5147
projectSlug,
5248
cloudApi,
49+
betterSqlite3Path: path.dirname(require.resolve('better-sqlite3/package.json')),
5350
})
5451

5552
this._studioHash = studioHash
@@ -72,6 +69,14 @@ export class StudioManager implements StudioManagerShape {
7269
return (await this.invokeAsync('canAccessStudioAI', { isEssential: true }, browser)) ?? false
7370
}
7471

72+
async initializeStudioAI (options: StudioAIInitializeOptions): Promise<void> {
73+
await this.invokeAsync('initializeStudioAI', { isEssential: true }, options)
74+
}
75+
76+
async destroy (): Promise<void> {
77+
await this.invokeAsync('destroy', { isEssential: true })
78+
}
79+
7580
private async reportError (error: Error): Promise<void> {
7681
try {
7782
const payload: StudioErrorReport = {

‎packages/server/lib/project-base.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -432,25 +432,32 @@ export class ProjectBase extends EE {
432432

433433
onStudioInit: async () => {
434434
if (this.spec && this.ctx.coreData.studio?.protocolManager) {
435-
const canAccessStudioAI = await this.ctx.coreData.studio?.canAccessStudioAI(this.browser) ?? false
435+
const canAccessStudioAI = await this.ctx.coreData.studio.canAccessStudioAI(this.browser) ?? false
436436

437437
if (!canAccessStudioAI) {
438438
return { canAccessStudioAI }
439439
}
440440

441-
this.protocolManager = this.ctx.coreData.studio?.protocolManager
442-
this.protocolManager?.setupProtocol()
443-
this.protocolManager?.beforeSpec({
441+
this.ctx.coreData.studio.protocolManager.setupProtocol()
442+
this.ctx.coreData.studio.protocolManager.beforeSpec({
444443
...this.spec,
445444
instanceId: v4(),
446445
})
447446

448-
await browsers.connectProtocolToBrowser({ browser: this.browser, foundBrowsers: this.options.browsers, protocolManager: this.protocolManager })
447+
await browsers.connectProtocolToBrowser({ browser: this.browser, foundBrowsers: this.options.browsers, protocolManager: this.ctx.coreData.studio.protocolManager })
449448

450-
if (this.protocolManager.db) {
451-
this.ctx.coreData.studio?.setProtocolDb(this.protocolManager.db)
449+
if (!this.ctx.coreData.studio.protocolManager.dbPath) {
450+
debug('Protocol database path is not set after initializing protocol manager')
451+
452+
return { canAccessStudioAI: false }
452453
}
453454

455+
this.protocolManager = this.ctx.coreData.studio.protocolManager
456+
457+
await this.ctx.coreData.studio.initializeStudioAI({
458+
protocolDbPath: this.ctx.coreData.studio.protocolManager.dbPath,
459+
})
460+
454461
return { canAccessStudioAI: true }
455462
}
456463

@@ -464,6 +471,7 @@ export class ProjectBase extends EE {
464471
await browsers.closeProtocolConnection({ browser: this.browser, foundBrowsers: this.options.browsers })
465472
this.protocolManager?.close()
466473
this.protocolManager = undefined
474+
await this.ctx.coreData.studio.destroy()
467475
}
468476
},
469477

‎packages/server/test/support/fixtures/cloud/studio/test-studio.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/// <reference types="cypress" />
22

33
import type { StudioServerShape, StudioServerDefaultShape } from '@packages/types'
4-
import type Database from 'better-sqlite3'
54
import type { Router } from 'express'
65
import type { Socket } from '@packages/socket'
76

@@ -14,11 +13,15 @@ class StudioServer implements StudioServerShape {
1413
return Promise.resolve(true)
1514
}
1615

17-
addSocketListeners (socket: Socket): void {
18-
// This is a test implementation that does nothing
16+
initializeStudioAI (): Promise<void> {
17+
return Promise.resolve()
1918
}
2019

21-
setProtocolDb (db: Database.Database): void {
20+
destroy (): Promise<void> {
21+
return Promise.resolve()
22+
}
23+
24+
addSocketListeners (socket: Socket): void {
2225
// This is a test implementation that does nothing
2326
}
2427
}

‎packages/server/test/unit/cloud/protocol_spec.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -350,17 +350,17 @@ describe('lib/cloud/protocol', () => {
350350
})
351351
})
352352

353-
describe('.db', () => {
354-
it('returns the database instance', () => {
355-
const mockDb = { test: 'db' }
353+
describe('.dbPath', () => {
354+
it('returns the database path', () => {
355+
const mockDbPath = '/path/to/db'
356356

357-
protocolManager['_db'] = mockDb
357+
protocolManager['_dbPath'] = mockDbPath
358358

359-
expect(protocolManager.db).to.equal(mockDb)
359+
expect(protocolManager.dbPath).to.equal(mockDbPath)
360360
})
361361

362-
it('returns undefined when no database is set', () => {
363-
expect(protocolManager.db).to.be.undefined
362+
it('returns undefined when no database path is set', () => {
363+
expect(protocolManager.dbPath).to.be.undefined
364364
})
365365
})
366366

‎packages/server/test/unit/cloud/studio_spec.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,15 +197,27 @@ describe('lib/cloud/studio', () => {
197197
})
198198
})
199199

200-
describe('setProtocolDb', () => {
201-
it('sets the protocol database on the studio server', () => {
202-
const mockDb = { test: 'db' }
200+
describe('initializeStudioAI', () => {
201+
it('initializes Studio AI on the studio server', async () => {
202+
sinon.stub(studio, 'initializeStudioAI').resolves()
203203

204-
sinon.stub(studio, 'setProtocolDb')
204+
await studioManager.initializeStudioAI({
205+
protocolDbPath: 'test-db-path',
206+
})
207+
208+
expect(studio.initializeStudioAI).to.be.calledWith({
209+
protocolDbPath: 'test-db-path',
210+
})
211+
})
212+
})
213+
214+
describe('destroy', () => {
215+
it('destroys the studio server', async () => {
216+
sinon.stub(studio, 'destroy').resolves()
205217

206-
studioManager.setProtocolDb(mockDb as any)
218+
await studioManager.destroy()
207219

208-
expect(studio.setProtocolDb).to.be.calledWith(mockDb)
220+
expect(studio.destroy).to.be.called
209221
})
210222
})
211223
})

‎packages/server/test/unit/project_spec.js

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ describe('lib/project-base', () => {
4343
this.testStudioManager = {
4444
initializeRoutes: () => {},
4545
status: 'INITIALIZED',
46+
destroy: () => Promise.resolve(),
4647
}
4748

4849
sinon.stub(studio, 'getAndInitializeStudioManager').resolves(this.testStudioManager)
@@ -736,18 +737,20 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
736737
it('passes onStudioInit callback with AI enabled and a protocol manager', async function () {
737738
const mockSetupProtocol = sinon.stub()
738739
const mockBeforeSpec = sinon.stub()
739-
const mockAccessStudioLLM = sinon.stub().resolves(true)
740-
const mockSetProtocolDb = sinon.stub()
740+
const mockAccessStudioAI = sinon.stub().resolves(true)
741+
const mockSetProtocolDbPath = sinon.stub()
742+
const mockInitializeStudioAI = sinon.stub().resolves()
741743

742744
this.project.spec = {}
743745
this.project.ctx.coreData.studio = {
744-
canAccessStudioAI: mockAccessStudioLLM,
746+
canAccessStudioAI: mockAccessStudioAI,
745747
protocolManager: {
746748
setupProtocol: mockSetupProtocol,
747749
beforeSpec: mockBeforeSpec,
748-
db: { test: 'db' },
750+
dbPath: 'test-db-path',
749751
},
750-
setProtocolDb: mockSetProtocolDb,
752+
setProtocolDbPath: mockSetProtocolDbPath,
753+
initializeStudioAI: mockInitializeStudioAI,
751754
}
752755

753756
sinon.stub(browsers, 'connectProtocolToBrowser').resolves()
@@ -783,7 +786,7 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
783786

784787
expect(mockSetupProtocol).to.be.calledOnce
785788
expect(mockBeforeSpec).to.be.calledOnce
786-
expect(mockAccessStudioLLM).to.be.calledWith({
789+
expect(mockAccessStudioAI).to.be.calledWith({
787790
family: 'chromium',
788791
name: 'chrome',
789792
channel: 'stable',
@@ -796,17 +799,66 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
796799
})
797800

798801
expect(this.project['_protocolManager']).to.eq(this.project.ctx.coreData.studio.protocolManager)
799-
expect(mockSetProtocolDb).to.be.calledWith({ test: 'db' })
802+
expect(mockInitializeStudioAI).to.be.calledWith({
803+
protocolDbPath: 'test-db-path',
804+
})
805+
})
806+
807+
it('handles case where protocol manager db path is not set', async function () {
808+
const mockSetupProtocol = sinon.stub()
809+
const mockBeforeSpec = sinon.stub()
810+
const mockAccessStudioAI = sinon.stub().resolves(true)
811+
const mockSetProtocolDbPath = sinon.stub()
812+
const mockInitializeStudioAI = sinon.stub().resolves()
813+
814+
this.project.spec = {}
815+
this.project.ctx.coreData.studio = {
816+
canAccessStudioAI: mockAccessStudioAI,
817+
protocolManager: {
818+
setupProtocol: mockSetupProtocol,
819+
beforeSpec: mockBeforeSpec,
820+
dbPath: null,
821+
},
822+
setProtocolDbPath: mockSetProtocolDbPath,
823+
initializeStudioAI: mockInitializeStudioAI,
824+
}
825+
826+
sinon.stub(browsers, 'connectProtocolToBrowser').resolves()
827+
828+
this.project.browser = {
829+
name: 'chrome',
830+
family: 'chromium',
831+
channel: 'stable',
832+
}
833+
834+
this.project.options.browsers = [{
835+
name: 'chrome',
836+
family: 'chromium',
837+
channel: 'stable',
838+
}]
839+
840+
let studioInitPromise
841+
842+
this.project.server.startWebsockets.callsFake(async (automation, config, callbacks) => {
843+
studioInitPromise = callbacks.onStudioInit()
844+
})
845+
846+
this.project.startWebsockets({}, {})
847+
848+
const { canAccessStudioAI } = await studioInitPromise
849+
850+
expect(canAccessStudioAI).to.be.false
851+
expect(this.project['_protocolManager']).to.be.undefined
800852
})
801853

802854
it('passes onStudioInit callback with AI enabled but no protocol manager', async function () {
803855
const mockSetupProtocol = sinon.stub()
804856
const mockBeforeSpec = sinon.stub()
805-
const mockAccessStudioLLM = sinon.stub().resolves(true)
857+
const mockAccessStudioAI = sinon.stub().resolves(true)
806858

807859
this.project.spec = {}
808860
this.project.ctx.coreData.studio = {
809-
canAccessStudioAI: mockAccessStudioLLM,
861+
canAccessStudioAI: mockAccessStudioAI,
810862
}
811863

812864
this.project.browser = {
@@ -836,20 +888,20 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
836888

837889
expect(mockSetupProtocol).not.to.be.called
838890
expect(mockBeforeSpec).not.to.be.called
839-
expect(mockAccessStudioLLM).not.to.be.called
891+
expect(mockAccessStudioAI).not.to.be.called
840892

841893
expect(browsers.connectProtocolToBrowser).not.to.be.called
842894
expect(this.project['_protocolManager']).to.be.undefined
843895
})
844896

845-
it('passes onStudioInit callback with llm disabled', async function () {
897+
it('passes onStudioInit callback with AI disabled', async function () {
846898
const mockSetupProtocol = sinon.stub()
847899
const mockBeforeSpec = sinon.stub()
848-
const mockAccessStudioLLM = sinon.stub().resolves(false)
900+
const mockAccessStudioAI = sinon.stub().resolves(false)
849901

850902
this.project.spec = {}
851903
this.project.ctx.coreData.studio = {
852-
canAccessStudioAI: mockAccessStudioLLM,
904+
canAccessStudioAI: mockAccessStudioAI,
853905
protocolManager: {
854906
setupProtocol: mockSetupProtocol,
855907
beforeSpec: mockBeforeSpec,
@@ -887,9 +939,11 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
887939

888940
it('passes onStudioDestroy callback', async function () {
889941
const mockClose = sinon.stub()
942+
const mockDestroy = sinon.stub().resolves()
890943

891944
this.project.ctx.coreData.studio = {
892945
protocolManager: {},
946+
destroy: mockDestroy,
893947
}
894948

895949
sinon.stub(browsers, 'closeProtocolConnection').resolves()
@@ -928,7 +982,7 @@ This option will not have an effect in Some-other-name. Tests that rely on web s
928982
})
929983

930984
expect(mockClose).to.be.calledOnce
931-
985+
expect(mockDestroy).to.be.calledOnce
932986
expect(this.project['_protocolManager']).to.be.undefined
933987
})
934988
})

0 commit comments

Comments
 (0)