Skip to content

Commit a37ba96

Browse files
author
Bhargava Varadharajan
committed
fix(smus): Fix potential race condition bug on getting context
**Description** We were seeing intermittent issues where sometimes the context was being returned as undefined. Setting it at the start of the activation to ensure the the context is available always. **Motivation** Bug fix **Testing Done** Tested manually locally.
1 parent 0401a71 commit a37ba96

File tree

2 files changed

+280
-0
lines changed

2 files changed

+280
-0
lines changed

packages/core/src/sagemakerunifiedstudio/activation.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@ import { activate as activateConnectionMagicsSelector } from './connectionMagics
88
import { activate as activateExplorer } from './explorer/activation'
99
import { isSageMaker } from '../shared/extensionUtilities'
1010
import { initializeResourceMetadata } from './shared/utils/resourceMetadataUtils'
11+
import { setContext } from '../shared/vscode/setContext'
12+
import { SmusUtils } from './shared/smusUtils'
1113

1214
export async function activate(extensionContext: vscode.ExtensionContext): Promise<void> {
1315
// Only run when environment is a SageMaker Unified Studio space
1416
if (isSageMaker('SMUS') || isSageMaker('SMUS-SPACE-REMOTE-ACCESS')) {
1517
await initializeResourceMetadata()
18+
// Setting context before any getContext calls to avoid potential race conditions.
19+
await setContext('aws.smus.inSmusSpaceEnvironment', SmusUtils.isInSmusSpaceEnvironment())
1620
await activateConnectionMagicsSelector(extensionContext)
1721
}
1822
await activateExplorer(extensionContext)
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import assert from 'assert'
7+
import sinon from 'sinon'
8+
import * as vscode from 'vscode'
9+
import { activate } from '../../sagemakerunifiedstudio/activation'
10+
import * as extensionUtilities from '../../shared/extensionUtilities'
11+
import * as connectionMagicsSelectorActivation from '../../sagemakerunifiedstudio/connectionMagicsSelector/activation'
12+
import * as explorerActivation from '../../sagemakerunifiedstudio/explorer/activation'
13+
import * as resourceMetadataUtils from '../../sagemakerunifiedstudio/shared/utils/resourceMetadataUtils'
14+
import { SmusUtils } from '../../sagemakerunifiedstudio/shared/smusUtils'
15+
16+
describe('SageMaker Unified Studio Main Activation', function () {
17+
let mockExtensionContext: vscode.ExtensionContext
18+
let isSageMakerStub: sinon.SinonStub
19+
let initializeResourceMetadataStub: sinon.SinonStub
20+
let setContextStub: sinon.SinonStub
21+
let isInSmusSpaceEnvironmentStub: sinon.SinonStub
22+
let activateConnectionMagicsSelectorStub: sinon.SinonStub
23+
let activateExplorerStub: sinon.SinonStub
24+
25+
beforeEach(function () {
26+
mockExtensionContext = {
27+
subscriptions: [],
28+
extensionPath: '/test/path',
29+
globalState: {
30+
get: sinon.stub(),
31+
update: sinon.stub(),
32+
},
33+
workspaceState: {
34+
get: sinon.stub(),
35+
update: sinon.stub(),
36+
},
37+
} as any
38+
39+
// Stub all dependencies
40+
isSageMakerStub = sinon.stub(extensionUtilities, 'isSageMaker')
41+
initializeResourceMetadataStub = sinon.stub(resourceMetadataUtils, 'initializeResourceMetadata')
42+
43+
// Use a safer approach for setContext to avoid "already wrapped" errors
44+
const setContextModule = require('../../shared/vscode/setContext')
45+
setContextStub = sinon.stub(setContextModule, 'setContext')
46+
47+
isInSmusSpaceEnvironmentStub = sinon.stub(SmusUtils, 'isInSmusSpaceEnvironment')
48+
activateConnectionMagicsSelectorStub = sinon.stub(connectionMagicsSelectorActivation, 'activate')
49+
activateExplorerStub = sinon.stub(explorerActivation, 'activate')
50+
51+
// Set default return values
52+
isSageMakerStub.returns(false)
53+
initializeResourceMetadataStub.resolves()
54+
setContextStub.resolves()
55+
isInSmusSpaceEnvironmentStub.returns(false)
56+
activateConnectionMagicsSelectorStub.resolves()
57+
activateExplorerStub.resolves()
58+
})
59+
60+
afterEach(function () {
61+
sinon.restore()
62+
})
63+
64+
describe('activate function', function () {
65+
it('should always activate explorer regardless of environment', async function () {
66+
isSageMakerStub.returns(false)
67+
68+
await activate(mockExtensionContext)
69+
70+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
71+
})
72+
73+
it('should not initialize SMUS components when not in SageMaker environment', async function () {
74+
isSageMakerStub.returns(false)
75+
76+
await activate(mockExtensionContext)
77+
78+
assert.ok(initializeResourceMetadataStub.notCalled)
79+
assert.ok(setContextStub.notCalled)
80+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
81+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
82+
})
83+
84+
it('should initialize SMUS components when in SMUS environment', async function () {
85+
isSageMakerStub.withArgs('SMUS').returns(true)
86+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
87+
isInSmusSpaceEnvironmentStub.returns(true)
88+
89+
await activate(mockExtensionContext)
90+
91+
assert.ok(initializeResourceMetadataStub.calledOnce)
92+
assert.ok(setContextStub.calledOnceWith('aws.smus.inSmusSpaceEnvironment', true))
93+
assert.ok(activateConnectionMagicsSelectorStub.calledOnceWith(mockExtensionContext))
94+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
95+
})
96+
97+
it('should initialize SMUS components when in SMUS-SPACE-REMOTE-ACCESS environment', async function () {
98+
isSageMakerStub.withArgs('SMUS').returns(false)
99+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(true)
100+
isInSmusSpaceEnvironmentStub.returns(false)
101+
102+
await activate(mockExtensionContext)
103+
104+
assert.ok(initializeResourceMetadataStub.calledOnce)
105+
assert.ok(setContextStub.calledOnceWith('aws.smus.inSmusSpaceEnvironment', false))
106+
assert.ok(activateConnectionMagicsSelectorStub.calledOnceWith(mockExtensionContext))
107+
assert.ok(activateExplorerStub.calledOnceWith(mockExtensionContext))
108+
})
109+
110+
it('should call functions in correct order for SMUS environment', async function () {
111+
isSageMakerStub.withArgs('SMUS').returns(true)
112+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
113+
isInSmusSpaceEnvironmentStub.returns(true)
114+
115+
await activate(mockExtensionContext)
116+
117+
// Verify the order of calls
118+
assert.ok(initializeResourceMetadataStub.calledBefore(setContextStub))
119+
assert.ok(setContextStub.calledBefore(activateConnectionMagicsSelectorStub))
120+
assert.ok(activateConnectionMagicsSelectorStub.calledBefore(activateExplorerStub))
121+
})
122+
123+
it('should handle initializeResourceMetadata errors', async function () {
124+
isSageMakerStub.withArgs('SMUS').returns(true)
125+
const error = new Error('Resource metadata initialization failed')
126+
initializeResourceMetadataStub.rejects(error)
127+
128+
await assert.rejects(() => activate(mockExtensionContext), /Resource metadata initialization failed/)
129+
130+
assert.ok(initializeResourceMetadataStub.calledOnce)
131+
assert.ok(setContextStub.notCalled)
132+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
133+
})
134+
135+
it('should handle setContext errors', async function () {
136+
isSageMakerStub.withArgs('SMUS').returns(true)
137+
isInSmusSpaceEnvironmentStub.returns(true)
138+
const error = new Error('Set context failed')
139+
setContextStub.rejects(error)
140+
141+
await assert.rejects(() => activate(mockExtensionContext), /Set context failed/)
142+
143+
assert.ok(initializeResourceMetadataStub.calledOnce)
144+
assert.ok(setContextStub.calledOnce)
145+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
146+
})
147+
148+
it('should handle connectionMagicsSelector activation errors', async function () {
149+
isSageMakerStub.withArgs('SMUS').returns(true)
150+
isInSmusSpaceEnvironmentStub.returns(true)
151+
const error = new Error('Connection magics selector activation failed')
152+
activateConnectionMagicsSelectorStub.rejects(error)
153+
154+
await assert.rejects(() => activate(mockExtensionContext), /Connection magics selector activation failed/)
155+
156+
assert.ok(initializeResourceMetadataStub.calledOnce)
157+
assert.ok(setContextStub.calledOnce)
158+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
159+
})
160+
161+
it('should handle explorer activation errors', async function () {
162+
const error = new Error('Explorer activation failed')
163+
activateExplorerStub.rejects(error)
164+
165+
await assert.rejects(() => activate(mockExtensionContext), /Explorer activation failed/)
166+
167+
assert.ok(activateExplorerStub.calledOnce)
168+
})
169+
170+
it('should pass correct extension context to all activation functions', async function () {
171+
isSageMakerStub.withArgs('SMUS').returns(true)
172+
isInSmusSpaceEnvironmentStub.returns(true)
173+
174+
await activate(mockExtensionContext)
175+
176+
assert.ok(activateConnectionMagicsSelectorStub.calledWith(mockExtensionContext))
177+
assert.ok(activateExplorerStub.calledWith(mockExtensionContext))
178+
})
179+
})
180+
181+
describe('environment detection logic', function () {
182+
it('should check both SMUS and SMUS-SPACE-REMOTE-ACCESS environments', async function () {
183+
isSageMakerStub.withArgs('SMUS').returns(false)
184+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
185+
186+
await activate(mockExtensionContext)
187+
188+
assert.ok(isSageMakerStub.calledWith('SMUS'))
189+
assert.ok(isSageMakerStub.calledWith('SMUS-SPACE-REMOTE-ACCESS'))
190+
})
191+
192+
it('should activate SMUS components if either environment check returns true', async function () {
193+
// Test case 1: Only SMUS returns true
194+
isSageMakerStub.withArgs('SMUS').returns(true)
195+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
196+
isInSmusSpaceEnvironmentStub.returns(true)
197+
198+
await activate(mockExtensionContext)
199+
200+
assert.ok(initializeResourceMetadataStub.calledOnce)
201+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
202+
203+
// Reset stubs for second test
204+
initializeResourceMetadataStub.resetHistory()
205+
activateConnectionMagicsSelectorStub.resetHistory()
206+
207+
// Test case 2: Only SMUS-SPACE-REMOTE-ACCESS returns true
208+
isSageMakerStub.withArgs('SMUS').returns(false)
209+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(true)
210+
isInSmusSpaceEnvironmentStub.returns(false)
211+
212+
await activate(mockExtensionContext)
213+
214+
assert.ok(initializeResourceMetadataStub.calledOnce)
215+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
216+
})
217+
218+
it('should use SmusUtils.isInSmusSpaceEnvironment() result for context setting', async function () {
219+
isSageMakerStub.withArgs('SMUS').returns(true)
220+
221+
// Test with true
222+
isInSmusSpaceEnvironmentStub.returns(true)
223+
await activate(mockExtensionContext)
224+
assert.ok(setContextStub.calledWith('aws.smus.inSmusSpaceEnvironment', true))
225+
226+
// Reset and test with false
227+
setContextStub.resetHistory()
228+
isInSmusSpaceEnvironmentStub.returns(false)
229+
await activate(mockExtensionContext)
230+
assert.ok(setContextStub.calledWith('aws.smus.inSmusSpaceEnvironment', false))
231+
})
232+
})
233+
234+
describe('integration scenarios', function () {
235+
it('should handle mixed success and failure scenarios gracefully', async function () {
236+
isSageMakerStub.withArgs('SMUS').returns(true)
237+
isInSmusSpaceEnvironmentStub.returns(true)
238+
239+
// initializeResourceMetadata succeeds, setContext fails
240+
const setContextError = new Error('Context setting failed')
241+
setContextStub.rejects(setContextError)
242+
243+
await assert.rejects(() => activate(mockExtensionContext), /Context setting failed/)
244+
245+
// Verify that initializeResourceMetadata was called but subsequent functions were not
246+
assert.ok(initializeResourceMetadataStub.calledOnce)
247+
assert.ok(setContextStub.calledOnce)
248+
assert.ok(activateConnectionMagicsSelectorStub.notCalled)
249+
assert.ok(activateExplorerStub.notCalled)
250+
})
251+
252+
it('should complete successfully when all components initialize properly', async function () {
253+
isSageMakerStub.withArgs('SMUS').returns(true)
254+
isSageMakerStub.withArgs('SMUS-SPACE-REMOTE-ACCESS').returns(false)
255+
isInSmusSpaceEnvironmentStub.returns(true)
256+
257+
// All functions should succeed
258+
await activate(mockExtensionContext)
259+
260+
// Verify all expected functions were called
261+
assert.ok(initializeResourceMetadataStub.calledOnce)
262+
assert.ok(setContextStub.calledOnce)
263+
assert.ok(activateConnectionMagicsSelectorStub.calledOnce)
264+
assert.ok(activateExplorerStub.calledOnce)
265+
})
266+
267+
it('should handle undefined extension context gracefully', async function () {
268+
const undefinedContext = undefined as any
269+
270+
// Should not throw for undefined context, but let the individual activation functions handle it
271+
await activate(undefinedContext)
272+
273+
assert.ok(activateExplorerStub.calledWith(undefinedContext))
274+
})
275+
})
276+
})

0 commit comments

Comments
 (0)