ci: don't fail "Publish test results" step when tests fail

[Copilot]

dorny/test-reporter was configured with fail-on-error: true, causing the "Publish test results" step to appear as the failing step even though the actual failure was already captured by dotnet test. The job correctly fails via the test execution steps — the reporter step adding a second failure is misleading noise.

Change

• .github/workflows/nightlybuild.yml: set fail-on-error: false on the dorny/test-reporter step

- name: Publish test results
  uses: dorny/test-reporter@v1
  if: always()
  with:
      name: Unit Test Results
      path: '**/TestResults/*.trx'
      reporter: dotnet-trx
      fail-on-error: false   # was: true

dotnet test still exits non-zero on test failure, failing the job as expected. The reporter step now always completes successfully and publishes results/annotations regardless of test outcome.

Original prompt

Create a pull request to adjust the GitHub Actions workflow so that the "Publish test results" step does not fail when tests fail, while preserving that the overall job fails when any test command fails.

Context:

• Repo: TransactionProcessing/TransactionProcessorACL
• Workflow run/job failing: https://github.com/TransactionProcessing/TransactionProcessorACL/actions/runs/25056992866/job/73399984193
• In .github/workflows/nightlybuild.yml, the step "Publish test results" uses dorny/test-reporter@v1 with fail-on-error: true, which causes the step to report failure when tests have failed.
• Requirement: the job should still fail if tests fail (i.e., keep dotnet test behavior unchanged), but the "Publish test results" step should not be marked as failed.

Requested change:

• Update .github/workflows/nightlybuild.yml to set fail-on-error: false for the "Publish test results" step (or remove the option).
• Do not modify the test execution steps such that they continue to fail the job on test failures.

Acceptance criteria:

• Workflow still fails when a test fails.
• "Publish test results" step completes successfully even when there are failing tests, and still publishes annotations/results.
• Keep existing if: always() so results are published even on failure.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The job is failing because an integration test (RedeemVouchers) cannot obtain an OAuth token for a seeded merchant user:

• Error: invalid_grant / "The username/password couple is invalid."
• Assertion: tokenResponseResult.IsSuccess was expected True but was False
• User/password used by the scenario: merchantuser@testmerchant1.co.uk / 123456
• Client used: merchantClient

This indicates the Security Service running in the integration-test docker stack does not recognize the credentials that the test believes it has just created (wrong password hashing/creation flow, wrong realm/tenant, wrong client, or the user isn’t actually being created in the correct environment).

Fix (most likely): ensure the integration tests create users with a password that matches the security service’s expected hashing/creation mechanism

Right now the test step “When I create the following security users” appears to create users, but the subsequent login fails as if the password was never set correctly. The most robust fix is to change the integration test user provisioning to set passwords via the same API path the Security Service uses, rather than directly inserting/bootstrapping users in a way that bypasses password hashing or policy.

Concrete code suggestion

In TransactionProcessorACL.IntegrationTests/Steps/SharedSteps.cs (the step that creates security users), ensure the user creation call:

1. Creates the user
2. Sets the password using the Security Service endpoint/mechanism intended for password setting (or supplies a properly supported password field during creation)

Pseudo-shape (adapt to your actual API client):

// After creating the user record:
await securityClient.CreateUserAsync(new CreateUserRequest {
    Email = email,
    GivenName = givenName,
    FamilyName = familyName,
    // do NOT rely on plain Password here if service ignores it
});

// Ensure password is set using supported flow
await securityClient.SetUserPasswordAsync(new SetPasswordRequest {
    Email = email,
    Password = password
});

If your security service requires an admin token to create users/set passwords, make sure the step acquires that admin token first and uses it for provisioning.

Fix (also common): align the client configuration used for password grant

The failing login uses client "merchantClient". If the Security Service no longer allows Resource Owner Password Credentials (password grant) for that client (or requires different scopes/audience), the service will respond with invalid_grant.

Make sure the integration environment seeds a client that:

• allows password grant (if you still rely on it)
• has the correct client_id/client_secret
• is created in the same Security Service instance the tests are calling

If you recently upgraded identity components, consider switching the integration tests from password grant to a supported flow (e.g., client credentials for machine-to-machine calls, or a test-only auth bypass), but the quickest unblock is to restore the expected "merchantClient" configuration in the integration test environment.

Make the failure easier to diagnose (immediate improvement)

Update the assertion site so the test prints the token endpoint URL, HTTP status code, and response body (it already prints body, but ensure status code + endpoint ...

This pull request was created from Copilot chat.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}