Skip to content

fix: write spector coverage by gracefully stopping server in conftest#4584

Open
msyyc wants to merge 4 commits into
mainfrom
msyyc/spector-graceful-stop-coverage
Open

fix: write spector coverage by gracefully stopping server in conftest#4584
msyyc wants to merge 4 commits into
mainfrom
msyyc/spector-graceful-stop-coverage

Conversation

@msyyc

@msyyc msyyc commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Stop the tsp-spector mock server gracefully from the Python e2e test run so that the spector coverage file (spec-coverage.json) is written without needing an extra step in the publish pipeline.

Background

tsp-spector only persists spec-coverage.json from its process exit handler, which is triggered by POST /.admin/stop (the admin route then calls process.exit(0)). Today the server is hard-killed when the parent process exits, which skips that handler — so no coverage file is produced. That is why #4579 adds a manual curl -X POST .../.admin/stop step in build-for-publish.yml.

Change

Add a pytest_unconfigure hook in packages/typespec-python/tests/conftest.py that gracefully stops the server via the admin endpoint:

  • Runs only in the xdist controller (or the non-xdist main process), i.e. after all workers finish, so the shared server is not killed prematurely while other workers still need it. This preserves the existing testserver fixture behavior that intentionally avoids stopping the server in per-worker teardown.
  • Waits until the server is no longer reachable, guaranteeing the synchronous coverage write has been flushed to disk before the upload step runs.
  • Is a safe no-op if the server is already stopped / was never started (mirrors the unconditional curl ... || true behavior it replaces).

With this in place, the extra pipeline step from #4579 is no longer needed.

Testing

Here is pipeline result against this PR: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6411100&view=logs&s=6884a131-87da-5381-61f3-d7acc3b91d76

And we could see the dashboard is updated as expected:
image

msyyc and others added 2 commits June 9, 2026 11:25
@msyyc
fix: gracefully stop spector server in conftest to write coverage
9480ff9 
The tsp-spector mock server only persists spec-coverage.json from its process exit handler, triggered by POST /.admin/stop. Previously the server was hard-killed on parent exit, skipping the coverage write and requiring an extra pipeline step. Add a pytest_unconfigure hook that gracefully stops the server (controller/non-xdist process only, after all workers finish) so coverage is written by the test run itself.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@msyyc
use tsp-spector server stop CLI and add chronus changelog
b955208 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@msyyc msyyc mentioned this pull request Jun 9, 2026
Closed
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:python Issues for @azure-tools/typespec-python emitter label Jun 9, 2026
@msyyc msyyc requested a review from Copilot June 9, 2026 03:44
@azure-sdk

azure-sdk commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

All changed packages have been documented.

  • ✅ @azure-tools/typespec-python
Show changes

@azure-tools/typespec-python - internal ✏️

Gracefully stop the spector mock server at the end of the test session so it writes its coverage file, removing the need for an extra pipeline step.

Copilot AI reviewed Jun 9, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to ensure tsp-spector writes spec-coverage.json as part of the Python e2e test run by gracefully stopping the shared mock server at the end of the pytest session (instead of relying on a separate pipeline curl step).

Changes:

  • Add graceful_stop_server() and a pytest_unconfigure hook in packages/typespec-python/tests/conftest.py to stop the mock server via tsp-spector server stop and wait for it to become unreachable.
  • Add a Chronus change entry documenting the internal testing/pipeline behavior improvement.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/typespec-python/tests/conftest.py Adds graceful server shutdown at end of pytest session to trigger spector coverage file persistence.
.chronus/changes/python-spector-coverage-graceful-stop-2026-6-9-11-30-0.md Records the internal change for the @azure-tools/typespec-python package.

Comment thread packages/typespec-python/tests/conftest.py
Comment thread packages/typespec-python/tests/conftest.py
Comment thread packages/typespec-python/tests/conftest.py
@pkg-pr-new

pkg-pr-new Bot commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@azure-tools/typespec-python@4584

commit: 9d7bee5

@github-actions

github-actions Bot commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

⚡ Benchmark Results

⚠️ 2 metric(s) regressed above the +5% threshold:

Metric Baseline Current Change
 ↳ emit/@azure-tools/typespec-autorest 🟢 171.9ms 🟡 204.3ms +18.8% 🔴
 ↳ emit/@typespec/openapi3/write 🟢 18.2ms 🟢 19.4ms +6.5% 🔴
Full details – comparing 7406622 vs baseline 91da400
Metric Baseline Current Change
total 🔴 683.1ms 🔴 641.7ms -6.1% 🟢
loader 🟡 227.7ms 🟢 189.0ms -17.0% 🟢
resolver 🟢 19.0ms 🟢 19.7ms +3.9%
checker 🟡 204.7ms 🟡 204.4ms -0.2%
validation 🟢 44.0ms 🟢 43.0ms -2.4%
 ↳ validation/@azure-tools/typespec-azure-core 🟢 7.1ms 🟢 7.0ms -1.5%
 ↳ validation/@typespec/http 🟢 6.7ms 🟢 6.5ms -2.4%
 ↳ validation/@typespec/rest 🟢 0.6ms 🟢 0.5ms -18.3%
 ↳ validation/@typespec/versioning 🔴 28.2ms 🔴 27.4ms -3.0%
 ↳ validation/compiler 🟢 1.6ms 🟢 1.5ms -7.9%
linter 🟢 143.8ms 🟢 142.9ms -0.6%
 ↳ linter/@azure-tools/typespec-azure-core/auth-required 🟢 0.0ms 🟢 0.0ms +4.8%
 ↳ linter/@azure-tools/typespec-azure-core/bad-record-type 🟢 0.2ms 🟢 0.2ms -5.0%
 ↳ linter/@azure-tools/typespec-azure-core/byos 🟢 6.1ms 🟢 6.2ms +0.7%
 ↳ linter/@azure-tools/typespec-azure-core/casing-style 🟢 0.7ms 🟢 0.7ms -3.4%
 ↳ linter/@azure-tools/typespec-azure-core/composition-over-inheritance 🟢 0.1ms 🟢 0.1ms -3.6%
 ↳ linter/@azure-tools/typespec-azure-core/documentation-required 🟢 0.9ms 🟢 0.8ms -3.7%
 ↳ linter/@azure-tools/typespec-azure-core/friendly-name 🟢 0.6ms 🟢 0.6ms -1.4%
 ↳ linter/@azure-tools/typespec-azure-core/key-visibility-required 🟢 0.2ms 🟢 0.2ms -2.6%
 ↳ linter/@azure-tools/typespec-azure-core/known-encoding 🟢 0.3ms 🟢 0.3ms -7.8%
 ↳ linter/@azure-tools/typespec-azure-core/long-running-polling-operation-required 🟢 0.3ms 🟢 0.3ms -2.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-case-mismatch 🟢 0.2ms 🟢 0.2ms +0.6%
 ↳ linter/@azure-tools/typespec-azure-core/no-closed-literal-union 🟢 0.3ms 🟢 0.3ms -0.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-enum 🟢 0.0ms 🟢 0.0ms -5.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-error-status-codes 🟢 0.1ms 🟢 0.1ms -2.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops 🟢 0.1ms 🟢 0.1ms -1.1%
 ↳ linter/@azure-tools/typespec-azure-core/no-format 🟢 0.6ms 🟢 0.6ms -7.1%
 ↳ linter/@azure-tools/typespec-azure-core/no-generic-numeric 🟢 0.4ms 🟢 0.4ms -2.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-header-explode 🔴 21.3ms 🔴 21.8ms +2.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-legacy-usage 🟢 1.1ms 🟢 1.1ms -1.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-multiple-discriminator 🟢 0.1ms 🟢 0.1ms -4.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-nullable 🟢 0.3ms 🟢 0.3ms +0.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-offsetdatetime 🟢 1.2ms 🟢 1.2ms -0.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-openapi 🟢 1.9ms 🟢 2.0ms +2.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-private-usage 🟢 2.0ms 🟢 1.8ms -8.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-query-explode 🔴 20.7ms 🔴 20.9ms +1.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-response-body 🔴 26.4ms 🔴 26.2ms -0.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-rest-library-interfaces 🟢 0.0ms 🟢 0.0ms -11.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch 🟢 5.3ms 🟢 5.4ms +1.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-rpc-path-params 🟢 0.2ms 🟢 0.2ms -1.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-string-discriminator 🟢 0.0ms 🟢 0.0ms -2.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-unknown 🟢 0.2ms 🟢 0.2ms -4.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-unnamed-union 🟢 0.4ms 🟢 0.3ms -3.1%
 ↳ linter/@azure-tools/typespec-azure-core/operation-missing-api-version 🟢 0.2ms 🟢 0.2ms -7.2%
 ↳ linter/@azure-tools/typespec-azure-core/request-body-problem 🟢 0.3ms 🟢 0.3ms -0.3%
 ↳ linter/@azure-tools/typespec-azure-core/require-versioned 🟢 0.0ms 🟢 0.0ms +5.4%
 ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem 🔴 25.0ms 🔴 23.3ms -6.8% 🟢
 ↳ linter/@azure-tools/typespec-azure-core/rpc-operation-request-body 🟢 0.4ms 🟢 0.3ms -3.8%
 ↳ linter/@azure-tools/typespec-azure-core/spread-discriminated-model 🟢 0.3ms 🟢 0.3ms -3.7%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-names 🟢 5.5ms 🟢 5.5ms -0.7%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-operations 🟢 0.1ms 🟢 0.1ms +4.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-common-types-version 🟢 4.2ms 🟢 4.1ms -1.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key 🟢 0.1ms 🟢 0.1ms +2.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage 🟢 0.1ms 🟢 0.1ms -6.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes 🟢 5.2ms 🟢 5.7ms +8.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts 🟢 4.6ms 🟢 4.4ms -3.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-record 🟢 0.4ms 🟢 0.4ms -0.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes 🟢 0.5ms 🟢 0.5ms +2.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes 🟢 0.0ms 🟢 0.0ms -1.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment 🟢 0.2ms 🟢 0.3ms +9.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property 🟢 0.1ms 🟢 0.1ms -3.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator 🟢 0.0ms 🟢 0.0ms -12.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb 🟢 0.1ms 🟢 0.1ms +10.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property 🟢 0.1ms 🟢 0.1ms -7.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format 🟢 0.0ms 🟢 0.0ms +4.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars 🟢 0.2ms 🟢 0.2ms -1.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern 🟢 0.0ms 🟢 0.0ms -21.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation 🟢 0.2ms 🟢 0.2ms -4.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response 🟢 4.7ms 🟢 4.7ms +0.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-patch 🟢 0.3ms 🟢 0.3ms -1.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars 🟢 0.2ms 🟢 0.2ms +4.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state 🟢 0.1ms 🟢 0.1ms -0.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels 🟢 0.1ms 🟢 0.1ms -1.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/empty-updateable-properties 🟢 0.2ms 🟢 0.1ms -10.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation 🟢 0.0ms 🟢 0.0ms +3.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header 🟡 15.4ms 🟡 14.4ms -6.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint 🟢 0.0ms 🟢 0.0ms -12.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers 🟢 0.3ms 🟢 0.3ms -8.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-empty-model 🟢 0.1ms 🟢 0.1ms -4.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-override-props 🟢 0.1ms 🟢 0.1ms -0.1%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation 🟢 0.2ms 🟢 0.2ms -1.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body 🔴 21.8ms 🔴 22.3ms +2.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/patch-envelope 🟢 0.1ms 🟢 0.1ms +3.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/resource-name 🟢 0.1ms 🟢 0.1ms -7.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/secret-prop 🟢 2.3ms 🟢 2.1ms -6.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/unsupported-type 🟢 0.4ms 🟢 0.4ms -7.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/version-progression 🟢 0.0ms 🟢 0.0ms +1.4%
 ↳ linter/@azure-tools/typespec-client-generator-core/property-name-conflict 🟢 1.1ms 🟢 1.1ms +1.5%
 ↳ linter/@azure-tools/typespec-client-generator-core/require-client-suffix 🟢 0.2ms 🟢 0.2ms -4.2%
emit 🔴 6.13s 🔴 6.16s +0.5%
 ↳ emit/@azure-tools/typespec-autorest 🟢 171.9ms 🟡 204.3ms +18.8% 🔴
 ↳ emit/@azure-tools/typespec-python 🔴 4.50s 🔴 4.52s +0.5%
 ↳ emit/@typespec/http-client-js 🔴 1.29s 🔴 1.18s -8.1% 🟢
 ↳ emit/@typespec/openapi3 🟢 153.5ms 🟢 156.3ms +1.8%
 ↳ emit/@typespec/openapi3/compute 🟢 135.6ms 🟢 137.2ms +1.2%
 ↳ emit/@typespec/openapi3/write 🟢 18.2ms 🟢 19.4ms +6.5% 🔴

Averaged across 3 specs (azure-arm-resource-manager, azure-core-dataplane, azure-full).
Threshold: changes > ±5% are highlighted.
🟢 Fast · 🟡 Moderate (stages >200ms, rules >10ms) · 🔴 Slow (stages >400ms, rules >20ms)

@azure-sdk

azure-sdk commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

You can try these changes here

🛝 Playground 🌐 Website

@msyyc msyyc mentioned this pull request Jun 9, 2026
Open
@msyyc msyyc enabled auto-merge June 9, 2026 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@iscai-msft iscai-msft Awaiting requested review from iscai-msft iscai-msft is a code owner

@tadelesh tadelesh Awaiting requested review from tadelesh tadelesh is a code owner

@l0lawrence l0lawrence Awaiting requested review from l0lawrence l0lawrence is a code owner

@ChenxiJiang333 ChenxiJiang333 Awaiting requested review from ChenxiJiang333 ChenxiJiang333 is a code owner

@bterlson bterlson Awaiting requested review from bterlson bterlson is a code owner

@markcowl markcowl Awaiting requested review from markcowl markcowl is a code owner

@timotheeguerin timotheeguerin Awaiting requested review from timotheeguerin timotheeguerin is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

emitter:python Issues for @azure-tools/typespec-python emitter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@msyyc @azure-sdk