fix(/dev): logic to allow customer to modify root source folder

[sannicm]

Problem

A recent refactor (#4924) moved collectFiles to a shared file but also updated the logic to throw another error that the /dev controller was not expecting. This effectively broke the feature that allows customer to modify root source folder when the folder exceeds 200MB

Solution

• Updated the if-else logic to check for the correct error
• Added a unit test to make it clearer that /dev controller expects the error to be specific to avoid breaking the logic.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[sannicm]

note to reviewers:

I've updated telemetry and added a new event emission to the /dev logic as the cost was very low and it was just one line in the logic. Let me know if you'd rather split this.

[kumsmrit]

If we modify this check, how will the ContentLengthError be handled that is being thrown explicitly here ?

[sannicm]

Good catch, I need to handle both.

[kumsmrit]

Instead of handling both, can we not revert this change back to throwing ContentLengthError in case of repo size being greater than 200MB ?

[kumsmrit]

Its better to keep existing check for ContentLengthError as well here.

[hayemaxi]

Why does the test need to run longer than 30 seconds?

[sannicm]

Becuase I'm trying to create files that will break the 200MB limit for repo size, this file creation and all takes more time than 30s

[sannicm]

I'm adding the test specifically to avoid any future regressions as a refactor made by another team broke our feature.

[hayemaxi]

This seems like a convoluted way of creating a large set of files. Can't you just try something like?

fs.writeFile('file', new Buffer(<some size>));

That way you can just make one big file, and it might be faster. Would like to avoid extending timeouts since tests already take a long time to run.

If so, make sure to use the fs implementation if srcShared/fs.ts

[hayemaxi]

Alternatively, maybe we can stub vscode.workspace.fs.stat(file.fileUri)) or something. I think that would be better than actually allocating a bunch of space on the testing machine (which would be our machines much of the time). I think this would be the preferred solution.

[justinmk3]

That way you can just make one big file, and it might be faster. Would like to avoid extending timeouts since tests already take a long time to run.

Yes, absolutely. In fact, this should be in testInteg/, that is where slow tests live.

[sannicm]

Thanks for the suggestions, I will take another look to this test then.

[sannicm]

I've resorted to stubbing the fsCommon.stat function. As there is no need in that case to make the test longer, by just stubbing the function I can return a value that will make the logic throw the error.

[justinmk3]

#4977 also changed the collectFiles() call in packages/core/src/amazonqFeatureDev/session/sessionState.ts , should that be done in this PR?

[sannicm]

#4977 also changed the collectFiles() call in packages/core/src/amazonqFeatureDev/session/sessionState.ts , should that be done in this PR?

Ahh I see @gasolima closed it, so I'll keep that feature here.

[justinmk3]

Does this collectFiles call need to be updated?

aws-toolkit-vscode/packages/core/src/amazonqFeatureDev/session/sessionState.ts

Lines 380 to 384 in df73086

	const files = await collectFiles(
	this.config.workspaceFolders.map(f => path.join(f.uri.fsPath, './mock-data')),
	this.config.workspaceFolders,
	false
	)