Add Spector test coverage for CommonPropertiesErrorClient methods#822
Add Spector test coverage for CommonPropertiesErrorClient methods#822
Conversation
…user_defined_error and get_for_predefined_error Co-authored-by: antkmsft <41349689+antkmsft@users.noreply.github.com>
Co-authored-by: antkmsft <41349689+antkmsft@users.noreply.github.com>
antkmsft
requested review from
RickWinter,
heaths and
jhendrixMSFT
as code owners
There was a problem hiding this comment.
Pull request overview
Adds Spector integration tests for the CommonPropertiesErrorClient operations in the common-properties ARM test crate, exercising the error paths for two previously untested client methods.
Changes:
- Adds a new integration test file covering
create_for_user_defined_error()andget_for_predefined_error(). - Verifies the returned
azure_core::Errorhas the expected HTTP status codes (400/404).
...pector/azure/resource-manager/common-properties/tests/common_properties_error_client_test.rs
Outdated
Show resolved
Hide resolved
...pector/azure/resource-manager/common-properties/tests/common_properties_error_client_test.rs
Outdated
Show resolved
Hide resolved
...pector/azure/resource-manager/common-properties/tests/common_properties_error_client_test.rs
Outdated
Show resolved
Hide resolved
...pector/azure/resource-manager/common-properties/tests/common_properties_error_client_test.rs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| const tspConfigPath = `${outputDir}/tspconfig.yaml`; |
There was a problem hiding this comment.
You can also set per-project additional args inline.
'spector_armcommon': {input: 'azure/resource-manager/common-properties', args: ['emit-error-types=true']},
| content += `${indent.get()}type Error = azure_core::Error;\n`; | ||
| content += `${indent.get()}fn try_from(error: azure_core::Error) -> std::result::Result<Self, Self::Error> {\n`; | ||
| content += `${indent.push().get()}match error.kind() {` | ||
| content += `${indent.push().get()}ErrorKind::HttpResponse { raw_response: Some(raw_response), .. } => Ok(serde_json::from_str(raw_response.body().clone().into_string()?.as_ref())?),`; |
There was a problem hiding this comment.
- Don't call
serde_jsondirectly. You can useraw_response.body().json()?even, or callazure_core::jsondirectly, but... - Why are you assuming JSON? Shouldn't this take into account the format the client actually uses, same as how the
F: Formatis determined forResponse<T, F>?
There was a problem hiding this comment.
You're also needless cloning the body. Deserializing just needs a reference. See https://azure.github.io/azure-sdk/rust_implementation.html for an example of a TryFrom for an error.
| type: 'boolean', | ||
| nullable: false, | ||
| default: false, | ||
| description: 'Whether to emit error types. Defaults to false' |
There was a problem hiding this comment.
I think we should always emit error models. Why not? TryFrom should be optional. In fact, we need it to be because what if they also want to take headers into account like Storage does? They'd need to implement TryFrom themselves.
| "async-trait", | ||
| "azure_core", | ||
| "serde", | ||
| "serde_json", |
There was a problem hiding this comment.
Not needed because you shouldn't be calling into serde_json directly anyway, nor even assuming JSON.
...st/test/spector/azure/resource-manager/common-properties/src/generated/models/models_impl.rs
Outdated
Show resolved
Hide resolved
…all sub-models recursively
Adds missing test coverage for
create_for_user_defined_error()andget_for_predefined_error()methods in CommonPropertiesErrorClient.Changes
common_properties_error_client_test.rsunderpackages/typespec-rust/test/spector/azure/resource-manager/common-properties/tests/common_properties_managed_identity_client_test.rs(FakeTokenCredential, helper functions, assertion structure)No modifications to generated source code or emitter.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.