Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/core/src/shared/sam/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,15 @@ export async function runSamSync(args: SyncParams) {
}),
})

await runInTerminal(sam, 'sync')
// with '--watch' selected, the sync process will run in the background until the user manually kills it
// we need to save the stack and region to the samconfig file now, otherwise the user would not see latest deployed resoure during this sync process
const { paramsSource, stackName, region, projectRoot } = args
const shouldWriteSyncSamconfigGlobal = paramsSource !== ParamsSource.SamConfig && !!stackName && !!region
if (boundArgs.includes('--watch')) {
shouldWriteSyncSamconfigGlobal && (await writeSamconfigGlobal(projectRoot, stackName, region))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Curious]: Shouldn't we just have the following line before await runInTerminal(sam, 'sync')?

shouldWriteSyncSamconfigGlobal && (await writeSamconfigGlobal(projectRoot, stackName, region))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could, if there's no specific reason why we added that line after running the command.

Copy link
Contributor

@vicheey vicheey Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding past commit here for historical context:

Auto refresh feature trigger extension to fetch for deployed stack node
using stack_name and region from samconfig.toml file. In current
logic, the stack_name and region are written before deployment
process completed to avoid the edge case when deployment failure due to
stack up-to-date. However, this order triggers the extension to fetch
for stack that may not already been created and through an error toast
message to customer stating "Stack does not exist".

Above is the reason we only save stack_name and region info after deploy/sync process completes. For the case of using --watch with sync, we can proceed with current approach.

}

await runInTerminal(sam, 'sync')
shouldWriteSyncSamconfigGlobal && (await writeSamconfigGlobal(projectRoot, stackName, region))
}

Expand Down
85 changes: 85 additions & 0 deletions packages/core/src/test/shared/sam/sync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,91 @@ describe('SAM runSync', () => {
prompterTester.assertCallAll()
})

it('[entry: command palette] specify and save flag (with --watch) should save params before starting SAM process', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

95% of the 100 lines in this test are identical to the test above it. The Copy-Paste Detection CI job warned about this. This is not acceptable in this codebase.

const prompterTester = PrompterTester.init()
.handleQuickPick('Select a SAM/CloudFormation Template', async (quickPick) => {
// Need sometime to wait for the template to search for template file
await quickPick.untilReady()
assert.strictEqual(quickPick.items[0].label, templateFile.fsPath)
quickPick.acceptItem(quickPick.items[0])
})
.handleQuickPick('Specify parameter source for sync', async (picker) => {
// Need time to check samconfig.toml file and generate options
await picker.untilReady()
assert.strictEqual(picker.items[0].label, 'Specify required parameters and save as defaults')
picker.acceptItem(picker.items[0])
})
.handleQuickPick('Select a region', (quickPick) => {
const select = quickPick.items.filter((i) => i.detail === 'us-west-2')[0]
quickPick.acceptItem(select || quickPick.items[0])
})
.handleQuickPick('Select a CloudFormation Stack', async (picker) => {
await picker.untilReady()
assert.strictEqual(picker.items[2].label, 'stack3')
picker.acceptItem(picker.items[2])
})
.handleQuickPick('Specify S3 bucket for deployment artifacts', async (picker) => {
await picker.untilReady()
assert.strictEqual(picker.items.length, 2)
assert.strictEqual(picker.items[0].label, 'Create a SAM CLI managed S3 bucket')
picker.acceptItem(picker.items[0])
})
.handleQuickPick('Specify parameters for sync', async (picker) => {
await picker.untilReady()
assert.strictEqual(picker.items.length, 9)
const dependencyLayer = picker.items.filter((item) => item.label === 'Dependency layer')[0]
const useContainer = picker.items.filter((item) => item.label === 'Use container')[0]
const watch = picker.items.filter((item) => item.label === 'Watch')[0]
picker.acceptItems(dependencyLayer, useContainer, watch)
})
.build()

// Invoke sync command from command palette
await runSync('code', undefined)

assert(mockGetSamCliPath.calledOnce)
assert(mockChildProcessClass.calledOnce)
assert.deepEqual(mockChildProcessClass.getCall(0).args, [
'sam-cli-path',
[
'sync',
'--code',
'--template',
`${templateFile.fsPath}`,
'--stack-name',
'stack3',
'--region',
'us-west-2',
'--no-dependency-layer',
'--save-params',
'--dependency-layer',
'--use-container',
'--watch',
],
{
spawnOptions: {
cwd: projectRoot?.fsPath,
env: {
AWS_TOOLING_USER_AGENT: 'AWS-Toolkit-For-VSCode/testPluginVersion',
SAM_CLI_TELEMETRY: '0',
},
},
},
])
assert(mockGetSpawnEnv.calledOnce)
assert(spyRunInterminal.calledOnce)
assert.deepEqual(spyRunInterminal.getCall(0).args, [mockSamSyncChildProcess, 'sync'])
assert.strictEqual(spyWriteSamconfigGlobal.callCount, 2)
assert(spyWriteSamconfigGlobal.calledBefore(spyRunInterminal))
// Check telementry
assertTelemetry('sam_sync', { result: 'Succeeded', source: undefined })
assertTelemetryCurried('sam_sync')({
syncedResources: 'CodeOnly',
source: undefined,
})
prompterTester.assertCallAll()
})

it('[entry: template file] specify flag should instantiate correct process in terminal', async () => {
const prompterTester = PrompterTester.init()
.handleQuickPick('Specify parameter source for sync', async (picker) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "appBuilder refresh feature doesnt work during sync --watch"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this description meaningful to customers?

}
Loading