Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ci] Avoid duplicating trx file on helix, and copy it for playground tests also #5253

Merged
merged 1 commit into from
Aug 13, 2024
Merged

[ci] Avoid duplicating trx file on helix, and copy it for playground tests also #5253

merged 1 commit into from
Aug 13, 2024

Conversation

radical
Copy link
Member

@radical radical commented Aug 10, 2024 •
edited
Loading

…tests also. These trx files are surfaced in the azdo artifacts that can be downloaded for debugging.

Description

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Aug 10, 2024
@radical radical added area-engineering-systems infrastructure helix infra engineering repo stuff testing ☑️ and removed area-integrations Issues pertaining to Aspire Integrations packages labels Aug 10, 2024
@radical radical marked this pull request as ready for review August 12, 2024 02:28
eerhardt
eerhardt reviewed Aug 12, 2024
View reviewed changes
tests/helix/send-to-helix-endtoendtests.targets
@@ -5,6 +5,11 @@
<NeedsWorkload>true</NeedsWorkload>
</PropertyGroup>

<ItemGroup>
<HelixPostCommand Condition="'$(OS)' != 'Windows_NT'" Include="cp $(_HelixLogsPath)/TestResults.trx $(_HelixLogsPath)/Aspire.EndToEnd.Tests-${TEST_SCENARIO}.trx" />
Copy link
Member

Choose a reason for hiding this comment

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

Is ${TEST_SCENARIO} and %TEST_SCENARIO% the right format here?

eerhardt
eerhardt approved these changes Aug 12, 2024
View reviewed changes
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM, if it works.

@radical radical merged commit e0e23d3 into dotnet:main Aug 13, 2024
11 checks passed
@radical radical deleted the helix-copy-trx branch August 13, 2024 04:21
radical added a commit to radical/aspire that referenced this pull request Aug 13, 2024
@radical
Squashed commit of the following:
015ec3e 
commit ed4c375
Merge: e676955 20d00e1
Author: Ankit Jain <radical@gmail.com>
Date:   Tue Aug 13 00:50:09 2024 -0400

    Merge remote-tracking branch 'origin/main' into re-enable-playground-tests

commit 20d00e1
Author: Ankit Jain <radical@gmail.com>
Date:   Tue Aug 13 00:49:21 2024 -0400

    [tests] Try to fix random failures on playground tests build (dotnet#5271)

    * [tests] Try to fix random failures on playground tests build

    `Aspire.Playground.Tests` build failed sometimes with errors like:

    `Could not copy "D:\a\_work\1\s\artifacts\obj\CatalogModel\Release\net8.0\CatalogModel.dll" to "D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll". Beginning retry 1 in 1000ms. The process cannot access the file 'D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll' because it is being used by another process. The file is locked by: "Pdb2Pdb (1628)"`

    The errors are not limited to this file, but can be one from any of the
    playground projects.

    Given:

    1. `Aspire.Playgroud.Tests` references various playground AppHost projects
       directly with `AdditionalProperties="SkipDashboardProjectReference=true"`.
    2. `Aspire.sln` also references these playground projects.

    My thinking is that msbuild ends up treating these projects like
    `TestShop/CatalogModel/CatalogModel.csproj` as two different project
    instances:

    1. One with `SkipDashboardProjectReference=true` (from
       `Aspire.Playground.Tests`)
    2. One without that property, when building the reference directly from
       `Aspire.sln`.

    This patch avoids that entirely by defaulting to
    `SkipDashboardProjectReference=true` on CI, and on helix.

    * Skip dashboard references

    * fix build

commit e676955
Author: Ankit Jain <radical@gmail.com>
Date:   Mon Aug 12 23:33:26 2024 -0400

    [tests] Re-enable playground tests

    .. and don't set `SkipDashboardProjectReference=true` (see
    dotnet#5271).

commit e0e23d3
Author: Ankit Jain <radical@gmail.com>
Date:   Tue Aug 13 00:21:37 2024 -0400

    [ci] Avoid duplicating trx file on helix, and copy it for playground tests also (dotnet#5253)

commit c3fbc5b
Author: Ankit Jain <radical@gmail.com>
Date:   Mon Aug 12 23:31:22 2024 -0400

    Squashed commit of the following:

    commit b24e087
    Merge: ee90475 4f1a352
    Author: Ankit Jain <radical@gmail.com>
    Date:   Mon Aug 12 23:11:22 2024 -0400

        Merge remote-tracking branch 'origin/main' into fix-playground-tests-build

        # Conflicts:
        #	tests/Aspire.Playground.Tests/Aspire.Playground.Tests.csproj

    commit ee90475
    Author: Ankit Jain <radical@gmail.com>
    Date:   Mon Aug 12 18:02:05 2024 -0400

        fix build

    commit e10b215
    Author: Ankit Jain <radical@gmail.com>
    Date:   Mon Aug 12 17:45:36 2024 -0400

        Skip dashboard references

    commit cf31acc
    Author: Ankit Jain <radical@gmail.com>
    Date:   Mon Aug 12 17:02:56 2024 -0400

        [tests] Try to fix random failures on playground tests build

        `Aspire.Playground.Tests` build failed sometimes with errors like:

        `Could not copy "D:\a\_work\1\s\artifacts\obj\CatalogModel\Release\net8.0\CatalogModel.dll" to "D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll". Beginning retry 1 in 1000ms. The process cannot access the file 'D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll' because it is being used by another process. The file is locked by: "Pdb2Pdb (1628)"`

        The errors are not limited to this file, but can be one from any of the
        playground projects.

        Given:

        1. `Aspire.Playgroud.Tests` references various playground AppHost projects
           directly with `AdditionalProperties="SkipDashboardProjectReference=true"`.
        2. `Aspire.sln` also references these playground projects.

        My thinking is that msbuild ends up treating these projects like
        `TestShop/CatalogModel/CatalogModel.csproj` as two different project
        instances:

        1. One with `SkipDashboardProjectReference=true` (from
           `Aspire.Playground.Tests`)
        2. One without that property, when building the reference directly from
           `Aspire.sln`.

        This patch avoids that entirely by defaulting to
        `SkipDashboardProjectReference=true` on CI, and on helix.

commit 42cc82d
Author: Sébastien Ros <sebastienros@gmail.com>
Date:   Mon Aug 12 20:24:22 2024 -0700

    Improve reliability of `AppHostTests.TestEndpointsReturnOk` (dotnet#5251)

    Use `WaitForText` since some services are really slow to start (CosmosDB emulator takes 45s locally).
    Use predicates in LoggerNotificationService since some texts are more complex than just `Contains`. For instance CosmonDBEmulator uses `Started` as the log when it's ready, but it also uses it many other times in the middle of some lines. Another example us MySql which starts the server twice (temp server).

    Disabling mongo tests - dotnet#5274

    * Improve reliability of `AppHostTests.TestEndpointsReturnOk`

    * Fix up usings

    * fix Aspire.Playground.Tests build

    * Format TestEndpoints

    * Remove unnecessary line

    * fix build

    * Improve mysql test reliability

    * Increase host startup timeout

    * Disable mongo app in playground tests - dotnet#5274

    ---------

    Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
    Co-authored-by: Ankit Jain <radical@gmail.com>

commit 990d6d0
Author: Ankit Jain <radical@gmail.com>
Date:   Mon Aug 12 23:12:54 2024 -0400

    playground.BrowserTelemetry: avoid using npm on ci/windows (dotnet#5269)

commit 4f1a352
Author: Ankit Jain <radical@gmail.com>
Date:   Mon Aug 12 22:38:17 2024 -0400

    [ci] Disable failing Aspire.Playground.Tests (dotnet#5273)

    * [ci] Disable failing Aspire.Playground.Tests

    There are two failures hitting CI right now:

    1. `Could not copy "D:\a\_work\1\s\artifacts\obj\CatalogModel\Release\net8.0\CatalogModel.dll" to "D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll". Beginning retry 1 in 1000ms. The process cannot access the file 'D:\a\_work\1\s\artifacts\bin\CatalogModel\Release\net8.0\CatalogModel.dll' because it is being used by another process. The file is locked by: "Pdb2Pdb (1628)"`

        - Exceptions like this when building playground apps.
        - Waiting on dotnet#5271 which might
          be a fix.

    2. Individual playground tests failing
        - Waiting on dotnet#5251

    This PR disable the tests completely to get the CI in a better state,
    and can be re-enabled once the aforementioned issues are fixed.

# Conflicts:
#	tests/Aspire.Playground.Tests/Aspire.Playground.Tests.csproj
@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eerhardt eerhardt eerhardt approved these changes

@RussKie RussKie Awaiting requested review from RussKie

@joperezr joperezr Awaiting requested review from joperezr

Assignees
No one assigned
Labels
area-engineering-systems infrastructure helix infra engineering repo stuff testing ☑️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@radical @eerhardt