SDK automation failed at migrate-package formatting step

[jeremymeng]

https://dev.azure.com/azure-sdk/internal/_build/results?buildId=4553716&view=logs&j=47092895-c7c8-567a-c7a6-8c88f3ec178e&t=d293a8e8-c6af-5091-cdf7-4c66482dd6f2&l=328

It looks that after generation we ran migrate-package command directly without running rush update so dev-tool failed to execute the npm run format command.

@wanlwanl I saw in some other automation run where typespec files was modified there's rush update before migrate-package. Do you know why it is not the same for the above run?

[wanlwanl]

It looks that after generation we ran migrate-package command directly without running rush update so dev-tool failed to execute the npm run format command.

I put migration step after rush update step for all sdk automations currently. Great, I can move migration step before rush update

[wanlwanl]

@jeremymeng We found the root cause, the migration tool failed 'cause test folder doesn't contain any test, then formatting step failed. Since ts emitter may config generateTest: false, there'll no tests.

Since it's hard to find root cause for dev tool, I created am issue for improving tsg experience for dev tool: #33196

[wanlwanl]

@qiaozha looks like we have case to set generateTest: false, so should we add generateTest: true in TypeSpecValidation Tool as planed?
https://github.com/Azure/azure-rest-api-specs/pull/32548/files#diff-2e54b0fdef5248e6dc9f92e7a5c8b476df062a712077dd42b865c90a414177b5

@jeremymeng could you share why you change generateTest to true? I'm making effort to add tspconfig.yaml validation rules to TypeSpec Validation tool, here's the loop you may be interested. The emitter side wants to generateTest: true for all azure packages, which is conflict with your settings

[wanlwanl]

Discussed in weekly meeting, js emitter will always generate tests, @MaryGao will create a related issue in emitter repo. Please confirm after codegen, the generate code always contains tests files.

If the generated sdk always contains tests, we don't have to change for sdk automation part.

Thanks @skywing918 for finding the root cause.

[jeremymeng]

@jeremymeng could you share why you change generateTest to true?

@wanlwanl @qiaozha @MaryGao I set generateTest to false because that particular package only exports types thus doesn't need any tests. Also for data-plane packages I could imagine that we probably don't need to regenerate tests every time.

[jeremymeng]

I put migration step after rush update step for all sdk automations currently. Great, I can move migration step before rush update

@wanlwanl It feels that we may need to run rush update twice? first one to ensure dependencies are there for dev-tool to run properly and second after the migration which changes the dependency list of the migrated package before we could build it and run its tests?

[wanlwanl]

I set generateTest to false because that particular package only exports types thus doesn't need any tests.
Also for data-plane packages I could imagine that we probably don't need to regenerate tests every time.

Sounds like migration tool's formatting should allow test folder to be empty.

It feels that we may need to run rush update twice?

It's already the current logic. You quote an out-of-date comment.

#32973 (comment) This is the current situation. Since there still a use case #32973 (comment), I think the best way is to let migration tool's formating command allows test folder to be empty.