Show neutral icon for container exiting with status code 0 #8477

adamint · 2025-04-01T19:56:16Z

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
  - Is this introducing a breaking change?
    - Yes
      - Link to aspire-docs issue (please use this breaking-change template):
    - No
      - Link to aspire-docs issue (please use this doc-idea template):
- No

Copilot

Pull Request Overview

This pull request introduces a change to display a neutral icon (RecordStop) for containers that exit with status code 0 in addition to finished containers. The changes include updating the inline test data to verify the new behavior, modifying the icon selection logic to support exited state with exit code 0, and adding a new helper method (IsExitedState) to support the updated condition.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/Aspire.Dashboard.Tests/Model/ResourceStateViewModelTests.cs	Adds inline test data for exited state with exit code 0 verifying the neutral icon and color.
src/Aspire.Dashboard/Model/ResourceStateViewModel.cs	Updates the conditional logic to include exited state when a valid exit code is present.
src/Aspire.Dashboard/Extensions/ResourceViewModelExtensions.cs	Introduces the IsExitedState method to clearly identify exited state resources.

src/Aspire.Dashboard/Model/ResourceStateViewModel.cs

JamesNK · 2025-04-02T02:00:55Z

src/Aspire.Dashboard/Model/ResourceStateViewModel.cs

@@ -42,7 +42,7 @@ private static (Icon icon, Color color) GetStateIcon(ResourceViewModel resource)
                icon = new Icons.Filled.Size16.ErrorCircle();
                color = Color.Error;
            }
-            else if (resource.IsFinishedState())
+            else if (resource.TryGetExitCode(out _) && (resource.IsFinishedState() || resource.IsExitedState()))


What is TryGetExitCode here for? Are you making it so there must be a known 0 exit code and if there isn't then show warning?

The concept of exit codes is specific to executable/containers. Other resource types could go into a finished/exited state without specifying one and then show a warning.

Please test these changes with resource types other than executables/containers.

For example,

davidfowl · 2025-04-02T03:46:57Z

I think we should unify exited and finished. I'm not sure why we have both.

adamint · 2025-04-03T20:12:31Z

I think we should unify exited and finished. I'm not sure why we have both.

@mitchdenny @karolz-ms ?

karolz-ms · 2025-04-03T22:39:27Z

In DCP Containers are Exited, and Executables are Finished at the end. It is just the way the world is 🤷‍♂️ 😄

Executables can also be Terminated, but that is related to replica sets which Aspire is not using anymore. And I have a bug to fix where Executables managed by an IDE go into Terminated state at the end; they should be Finished instead. It was supposed to be fixed in 9.2 but I ran out of time.

JamesNK · 2025-04-03T23:32:09Z

I'm ok with making them consistent. Exited would be the logic choice for use for both. And it lines up with exit codes.

* Show neutral icon for container exiting with status code 0 * Show info/stop icons for all exited resources with status code 0 * Add finished test for custom resource, remove exit code expectation * update tests

* Migrate from VSTest to Microsoft.Testing.Platform * Address TODO * Cleanup stuff around runsettings * Add comment * Progress * Remove locale * Always show live output * Restore test session timeout * Fix timeout * Fix typo * Fix duplicate commnand-line options * Fix Linux * Fix for Linux * Fix * Fix * Filter failing * Fix * ignore exit code 8 * Fix extra dot * Ignore exit code * Fix duplicate --ignore-exit-code * Fix playground tests on helix * Move to RepoTesting.props * Fix results dir * Fix * Progress * Fix duplicate command-line option * Missing report-trx * Run executable * Address review comments * Revert "Run executable" This reverts commit d10e168. * dotnet dll * Introduce version selector for Aspire templates (#8625) * Introduce version selecto to aspire new * Update src/Aspire.Cli/Commands/NewCommand.cs Co-authored-by: Copilot <[email protected]> * Update src/Aspire.Cli/Commands/NewCommand.cs Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: David Fowler <[email protected]> Co-authored-by: Copilot <[email protected]> * Error handling for GetCapabilitiesAsync connection issues (#8614) * Error out when GetCapabiltiesAsync is missing or unexpected exception occurs. * Update src/Aspire.Cli/DotNetCliRunner.cs Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]> * Obsolete AddAzureContainerAppsInfrastructure (#8639) * Obsolete AddAzureContainerAppsInfrastructure This method is no longer meant to be used. Instead developers should be calling AddAzureContainerAppEnvironment. Covert the tests using this API to the new API * Fix volume output naming issue We weren't discriminating between volumes and bindmounts, which caused a cache collision. * Switch the prefix at the beginning * Add error handling for package updates in workflow (#8390) * Filter out tracing health endpoints in service defaults templates (#8643) * Filter out tracing health endpoints Fixes #8580 * Fix typo * Show neutral icon for container exiting with status code 0 (#8477) * Show neutral icon for container exiting with status code 0 * Show info/stop icons for all exited resources with status code 0 * Add finished test for custom resource, remove exit code expectation * update tests * [tests] Correctly mark Aspire.Dashboard.Tests.Integration.Playwright.AppBarTests with RequiresPlaywright (#8647) * [tests] Skip generating Directory.Packages.Versions.props for Test (#8621) .. Utility projects. * Fix running playground tests on windows --------- Co-authored-by: Ankit Jain <[email protected]> Co-authored-by: Dan Moseley <[email protected]> Co-authored-by: Mitch Denny <[email protected]> Co-authored-by: David Fowler <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Eric Erhardt <[email protected]> Co-authored-by: Jose Perez Rodriguez <[email protected]> Co-authored-by: Damian Edwards <[email protected]> Co-authored-by: Adam Ratzman <[email protected]>

adamint added 2 commits April 1, 2025 14:28

Show neutral icon for container exiting with status code 0

9732674

Show info/stop icons for all exited resources with status code 0

f18f4a0

Copilot AI review requested due to automatic review settings April 1, 2025 19:56

github-actions bot added the area-dashboard label Apr 1, 2025

Copilot AI reviewed Apr 1, 2025

View reviewed changes

src/Aspire.Dashboard/Model/ResourceStateViewModel.cs Outdated Show resolved Hide resolved

JamesNK reviewed Apr 2, 2025

View reviewed changes

adamint added 2 commits April 3, 2025 16:18

Add finished test for custom resource, remove exit code expectation

00d2ca4

update tests

578c616

adamint requested a review from JamesNK April 3, 2025 20:22

JamesNK approved these changes Apr 4, 2025

View reviewed changes

adamint merged commit 3781c4a into dotnet:main Apr 8, 2025
174 checks passed

github-actions bot locked and limited conversation to collaborators May 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Show neutral icon for container exiting with status code 0 #8477

Show neutral icon for container exiting with status code 0 #8477

adamint commented Apr 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

JamesNK Apr 2, 2025 •

edited

Loading

Uh oh!

JamesNK Apr 2, 2025

Uh oh!

davidfowl commented Apr 2, 2025

Uh oh!

adamint commented Apr 3, 2025

Uh oh!

karolz-ms commented Apr 3, 2025

Uh oh!

JamesNK commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

Show neutral icon for container exiting with status code 0 #8477

Show neutral icon for container exiting with status code 0 #8477

Conversation

adamint commented Apr 1, 2025

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

JamesNK Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JamesNK Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

davidfowl commented Apr 2, 2025

Uh oh!

adamint commented Apr 3, 2025

Uh oh!

karolz-ms commented Apr 3, 2025

Uh oh!

JamesNK commented Apr 3, 2025

Uh oh!

Uh oh!

Uh oh!

JamesNK Apr 2, 2025 •

edited

Loading