Skip to content

ManualTestCreate: add TestEnvironmentId (Tests.CreateAsync always 400'd) - #25

Open
jochen-testingbot wants to merge 1 commit into
mainfrom
fix/manual-test-environment-id
Open

jochen-testingbot wants to merge 1 commit into
mainfrom
fix/manual-test-environment-id

Conversation

@jochen-testingbot

Copy link
Copy Markdown

Part of https://testingbot-team.atlassian.net/browse/TB-1008 (docs audit: run every SDK README snippet).

README walkthrough (live API; .NET SDK 9.0.320, net8.0 console app)

The calls the README shows all work: FromEnvironment, User.GetAsync, Tests.ListAsync/ListAllAsync/GetAsync/UpdateAsync (README form with groups)/DeleteAsync, TestingBotNotFoundException, Browsers.ListAsync, Storage.UploadAsync(FileInfo, progress)/ReplaceAsync(AppKey, FileInfo)/DeleteAsync, Builds, Devices, Tunnels, Team.GetConcurrencyAsync, Configuration.GetIpRangesAsync and GetSharingAuthHash.
(ListAsync(Build = "ci-1421") 404s only because the build doesn't exist yet. That is expected.)

Bug fixed here

Tests.CreateAsync(new ManualTestCreate { ... }) can never succeed:

TestingBotValidationException: POST https://api.testingbot.com/v1/tests: test[test_environment_id] is missing

ManualTestCreate had no field for it. The web API reference leaves out the C# tab for this reason (config/api_examples/tests.yml).

Changes

  • ManualTestCreate.TestEnvironmentId (int?), sent as test[test_environment_id].
  • Unit test Tests_Create_sends_test_environment_id.
  • README: a short "Recording an external result" snippet that uses browsers[0].BrowserId.

Verification

dotnet test tests/TestingBot.Api.Tests   -> Passed! 124/124 (net9.0)
Tests.CreateAsync(... TestEnvironmentId = browsers[0].BrowserId ...)  -> true   (without it: TestingBotValidationException, as above)
walkthrough SUMMARY 18 ok / 20 (the two failures are the build that doesn't exist yet and create without the new field)

I deleted the test, storage entry and builds the run created.

Also noticed: the quick start prints user.Email, but GET /v1/user returns no email for this account (it prints empty). Not changed here.

Release needed on NuGet (TestingBot.Api 1.0.1). That is needs-jochen.

Tests.CreateAsync always failed with TestingBotValidationException
'test[test_environment_id] is missing': the record had no way to carry
the field. Add it, send it as test[test_environment_id], cover it with a
unit test and show it in the README.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new API-required field remains nullable and optional, allowing callers to omit it and still receive a 400 response instead of catching the omission at compile time.

1 open issue
What changed in this PR

This pull request fixes manual test creation by adding and submitting the required test-environment identifier, adds regression coverage, and documents the external-result workflow.

Changes:

  • Adds ManualTestCreate.TestEnvironmentId and sends it as test[test_environment_id].
  • Adds a unit test verifying the new form field.
  • Adds a README example using a browser’s BrowserId.
  • The new field remains nullable even though the API requires it; this should be made non-nullable to prevent requests that still fail with HTTP 400.
File Summary
tests/​TestingBot.Api.Tests/​Clients/​WriteClientsTests.cs Verifies that manual test creation sends the new environment identifier field.
src/​TestingBot.Api/​Clients/​TestsClient.Writes.cs Includes the environment identifier in the manual test creation POST form payload.
src/​TestingBot.Api/​Clients/​Requests.cs Defines the manual test environment identifier request property.
README.md Documents recording an external test result using a browser environment ID.

This pull request requires changes.

Review effort: Lite

/// The <c>browser_id</c> of the browser/device the test ran on (from <c>client.Browsers</c>).
/// The API rejects the request without it ("test[test_environment_id] is missing").
/// </summary>
public int? TestEnvironmentId { get; init; }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not changing this in this PR. Adding a required member to the public ManualTestCreate record breaks the source of every existing caller, which would need a major version, while this fix is meant to ship as a patch (1.0.1). The property is documented as required by the API in its XML doc and in the README, and the server's 400 message names the field. Making it required in a future major release is reasonable; I've noted that for the maintainer on TB-1008.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants