Skip to content

Commit 3e41b0b

Browse files
Improve CONTRIBUTING.md (#2258)
* Improve CONTRIBUTING.md Resolves #2012 and resolves #2109. * Resolve PR feedback Co-authored-by: Ronnie Geraghty <[email protected]> --------- Co-authored-by: Ronnie Geraghty <[email protected]>
1 parent 725f1bf commit 3e41b0b

File tree

2 files changed

+104
-42
lines changed

2 files changed

+104
-42
lines changed

CONTRIBUTING.md

Lines changed: 84 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,79 @@
1010

1111
## Generated code
1212

13-
If you want to contribute to a file that is generated (the file is located in a `generated` subdirectory), the best approach is to open a PR on the TypeSpec specification since we cannot replace generated code that will be replaced when regenerated. Please visit the [Azure/azure-rest-api-specs repo](https://github.com/Azure/azure-rest-api-specs/) to view and make changes to Azure service API specifications.
13+
If you want to contribute to a file that is generated (the file is located in a `generated` subdirectory), the best approach is to open a PR on the TypeSpec specification since we cannot replace generated code that will be replaced when regenerated.
14+
Please visit the [Azure/azure-rest-api-specs repo](https://github.com/Azure/azure-rest-api-specs/) to view and make changes to Azure service API specifications.
15+
16+
## Coding
17+
18+
We welcome contributions! But before you start coding, please read our [Rust Guidelines] including [implementation details](https://azure.github.io/azure-sdk/rust_implementation.html) for contributors.
19+
20+
Most of our code is generated from [TypeSpec] and, more specifically for Azure services, [TypeSpec Azure] libraries. Changes to any services - including documentation improvements - need to made in TypeSpec specifications
21+
found in <https://github.com/Azure/azure-rest-api-specs>. Only TypeSpec is supported at this time, so any changes to OpenAPI v2 a.k.a., "swagger", specifications will not be used.
22+
23+
Crates containing the majority of written code include `azure_core` and its dependencies. Some crates like `azure_security_keyvault_secrets` might also have some written code to improve usability.
1424

1525
## Building
1626

17-
To build any library in the Azure SDK for Rust navigate to the library's project folder and run `cargo build`.
27+
To build any crate in the Azure SDK for Rust navigate to the crate's project folder and run `cargo build`.
28+
Alternatively, you can build any one or more crates by passing their crate names to `--package` (short: `-p`) e.g., `cargo build -p azure_security_keyvault_secrets`.
29+
30+
You can also build the entire workspace by either building from the root source directory or running `cargo build --workspace`, but unless you're making changes to `azure_core`
31+
or its dependencies, this is generally unnecessary nor recommended. It will take considerable time and drive space.
32+
33+
### Linting
34+
35+
You can run `cargo clippy` to check for common issues. Like `cargo build`, you can pass one or more crate names to `--package`.
36+
Before create a pull request (PR), it's a good practice to build and lint your project to avoid a lot of commits that can make the review process tedious for reviewers.
1837

1938
## Testing
2039

21-
[TODO] Add instructions on how to run tests for a specific project.
22-
[TODO] Add instructions for write new tests.
40+
To test any crate in the Azure SDK for Rust navigate to the crate's project folder and run `cargo test`.
41+
Alternatively, you can test any one or more crates by passing their crate names to `--package` (short: `-p`) e.g., `cargo test -p azure_security_keyvault_secrets`.
42+
43+
This command will run all tests in the selected packages, including unit tests, integration tests, any tests within examples, and doc tests.
44+
To learn more about the different styles of tests and where they are located in a project, see [Tests in The Cargo Book](https://doc.rust-lang.org/cargo/reference/cargo-targets.html#tests).
45+
46+
### Integration Tests
47+
48+
We use integration tests - tests defined under a crate's `tests/` directory - for testing against provisioned resources.
49+
Most crates use recorded tests to record (and sanitize) or play back HTTP traffic during test execution by attributing tests with `#[recorded::test]`.
50+
51+
If your crate does not communicate over HTTP or provisioning resources cannot be fully automated, you can also mark tests as `#[recorded::test(live)]`.
52+
53+
For more details about recorded tests, please refer to the [`azure_core_test` crate](https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/core/azure_core_test/README.md).
54+
55+
### Documentation Tests
56+
57+
Though you should use integration tests for testing client methods, or create examples under `examples/` that show customers how to do common tasks,
58+
[documentation tests](https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html) can be helpful to show customers examples of calling your code
59+
e.g., utility code to parse some resource identifier, that are also runnable (or just compilable) and will fail `cargo test` if the test assertions fail.
60+
61+
Each crate should define document tests to highlight common scenarios. Unlike markdown comments in Rust `.rs` source code, however, you need to define the code fence
62+
using the `rust no_run` declaration e.g.,
63+
64+
````markdown
65+
```rust no_run
66+
#[tokio::main]
67+
fn main() -> Result<(), Box<dyn std::error::Error>> {
68+
todo!()
69+
}
70+
```
71+
````
72+
73+
This is because when GitHub renders the `README.md` file, it needs to know the syntax. The `no_run` tells `cargo test` to only compile the test - to make sure it indeed compiles - but not to run it,
74+
which is important if it shows an example running against live resources that likely include placeholder values e.g., `https://my-vault.vault.azure.net`.
75+
76+
More guidance for writing documentation tests can be found in our [Rust Guidelines].
2377

2478
### Debugging with Visual Studio Code
2579

2680
[Visual Studio Code] with recommended extensions installed can be used to run and debug tests for a module or individual tests.
81+
In most cases you can click **Run Test** or **Debug** found right above the test function.
82+
For integration tests - those tests found under the `tests/` directory - this will run or debug the test using a previously recorded session, if one exists.
2783

28-
If you need to debug a test, you can use the LLDB extension and set environment variables as needed. For example, to debug recording a specific test,
29-
your `.vscode/launch.json` file might look something like:
84+
If a recording for an integration test does not exist or needs to be updated, along with the LLDB extension installed
85+
you can create or update a `.vscode/launch.json` file to debug recording a session e.g.:
3086

3187
```json
3288
{
@@ -49,47 +105,38 @@ your `.vscode/launch.json` file might look something like:
49105
"kind": "test"
50106
},
51107
"env": {
108+
"AZURE_TEST_MODE": "record"
52109
}
53110
},
54111
"cwd": "${workspaceFolder}",
55112
"env": {
56113
"AZURE_KEYVAULT_URL": "https://my-vault.vault.azure.net/",
57-
"PROXY_MANUAL_START": "true",
58-
"RUST_LOG": "trace"
59-
}
60-
},
61-
{
62-
"type": "lldb",
63-
"request": "launch",
64-
"name": "Play back secret_roundtrip",
65-
"cargo": {
66-
"args": [
67-
"test",
68-
"--no-run",
69-
"--test=secret_client",
70-
"--package=azure_security_keyvault_secrets",
71-
"secret_roundtrip"
72-
],
73-
"filter": {
74-
"name": "secret_client",
75-
"kind": "test"
76-
},
77-
"env": {
78-
"AZURE_TEST_MODE": "playback"
79-
}
80-
},
81-
"cwd": "${workspaceFolder}",
82-
"env": {
83114
"RUST_LOG": "trace"
84115
}
85116
}
86117
]
87118
}
88119
```
89120

90-
You can also start the [Test Proxy] manually, in which can you add to the outer `env` above to `"PROXY_MANUAL_START": "true"`.
121+
If you don't need to debug recording the test or running live, you can record from the command line e.g.:
122+
123+
```sh
124+
AZURE_KEYVAULT_URL=https://my-vault.vault.azure.net AZURE_TEST_MODE=record cargo test -p azure_security_keyvault_secrets --test secret_client
125+
```
126+
127+
In either case above, the [Test Proxy] will be started automatically if installed.
128+
129+
#### Running Test Proxy Manually
130+
131+
You can also start the [Test Proxy] manually, which may provide additional diagnostics.
132+
133+
If you're using a `.vscode/launch.json` file, in the outer `env` above add `"PROXY_MANUAL_START": "true"`.
134+
135+
Similarly, if running on the command line pass `PROXY_MANUAL_START=true`.
136+
137+
#### Enabling Trace Logging
91138

92-
To enable tracing, you can add the `RUST_LOG` environment variable as shown above using the [same format supported by `env_logger`](https://docs.rs/env_logger/latest/env_logger/#enabling-logging).
139+
To log tracing information to the terminal, you can add the `RUST_LOG` environment variable as shown above using the [same format supported by `env_logger`](https://docs.rs/env_logger/latest/env_logger/#enabling-logging).
93140
The targets are the crate names if you want to trace more or less for specific targets e.g., `RUST_LOG=info,azure_core=trace` to trace information messages by default but detailed traces for the `azure_core` crate.
94141

95142
## Code Review Process
@@ -166,5 +213,8 @@ Samples may take the following categories of dependencies:
166213

167214
In general, we prefer taking dependencies on licensed components in the order of the listed categories. In cases where the category may not be well known, we'll document the category so that readers understand the choice that they're making by using that dependency.
168215

216+
[Rust Guidelines]: https://azure.github.io/azure-sdk/rust_introduction.html
169217
[Test Proxy]: https://github.com/Azure/azure-sdk-tools/blob/main/tools/test-proxy/Azure.Sdk.Tools.TestProxy/README.md
218+
[TypeSpec]: https://aka.ms/typespec
219+
[TypeSpec Azure]: https://aka.ms/typespec/azure
170220
[Visual Studio Code]: https://code.visualstudio.com

sdk/core/azure_core_test/README.md

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
# Azure client library test utilities
22

3-
The types and functions in this crate help test client libraries built on `azure_core`.
3+
The types and functions in this crate help test client libraries built on `azure_core`, including the `#[recorded::test]` attribute.
4+
5+
We use integration tests - tests defined under a crate's `tests/` directory - for testing against provisioned resources.
6+
Most crates use recorded tests to record (and sanitize) or play back HTTP traffic during test execution by attributing tests with `#[recorded::test]`.
7+
8+
If your crate does not communicate over HTTP or provisioning resources cannot be fully automated, you can also mark tests as `#[recorded::test(live)]`.
9+
This still provides utility such as reading environment variables or other test context that might be helpful when your tests run.
10+
11+
## Prerequisites
12+
13+
- [Test Proxy]
14+
- (Recommended) [PowerShell] to easily provision resources
415

516
## Client methods
617

@@ -43,15 +54,15 @@ These tests must also return a `std::result::Result<T, E>`, which can be redefin
4354
Besides instrumenting your client and getting credentials - real credentials when running live or recording,
4455
but mock credentials when playing back - there are a number of other helpful features on the `Recording` object returned above:
4556

46-
* `add_sanitizer` will add custom sanitizers. There are many pre-configured by the [Test Proxy] as well.
47-
* `remove_sanitizers` will remove named sanitizers, like `AZSDK3430` that sanitizes all `$..id` fields and may cause playback to fail.
48-
* `add_matcher` adds a custom matcher to match headers, path segments, and or body content.
49-
* `random` gets random data (numbers, arrays, etc.) that is initialized from the OS when running live or recording,
57+
- `add_sanitizer` will add custom sanitizers. There are many pre-configured by the [Test Proxy] as well.
58+
- `remove_sanitizers` will remove named sanitizers, like `AZSDK3430` that sanitizes all `$..id` fields and may cause playback to fail.
59+
- `add_matcher` adds a custom matcher to match headers, path segments, and or body content.
60+
- `random` gets random data (numbers, arrays, etc.) that is initialized from the OS when running live or recording,
5061
but the seed is saved with the recording and used during play back so that sequential generation of random data is deterministic.
5162
ChaCha20 is used to provide a deterministic, portable sequence of seeded random data.
52-
* `var` gets a required variable with optional `ValueOptions` you can use to sanitize values.
63+
- `var` gets a required variable with optional `ValueOptions` you can use to sanitize values.
5364
This function will err if the variable is not set in the environment when running live or recording, or available when playing back.
54-
* `var_opt` gets optional variables and will not err in the aforementioned cases.
65+
- `var_opt` gets optional variables and will not err in the aforementioned cases.
5566

5667
## Record tests
5768

@@ -61,7 +72,7 @@ This uses a `test-resources.json` or `test-resources.bicep` file in the crate or
6172
When you run this, it will output some environment variables you can set in your shell, or pass on the command line if your shell supports it.
6273

6374
```bash
64-
pwsh ./eng/common/TestResources/New-TestResources.ps1 -ServiceDirectory keyvault
75+
./eng/common/TestResources/New-TestResources.ps1 -ServiceDirectory keyvault
6576
```
6677

6778
In this example, it will output a number of environment variables including `AZURE_KEYVAULT_URL`. We'll pass that in the command line for bash below.
@@ -94,5 +105,6 @@ cargo test
94105
If you get errors, they could indicate regressions in your tests or perhaps variables or random data wasn't saved correctly.
95106
Review any data you generate or use not coming from the service.
96107

108+
[PowerShell]: https://learn.microsoft.com/powershell/scripting/install/installing-powershell
97109
[Test Proxy]: https://github.com/Azure/azure-sdk-tools/blob/main/tools/test-proxy/Azure.Sdk.Tools.TestProxy/README.md
98110
[Test Resources]: https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/TestResources/README.md

0 commit comments

Comments
 (0)