Skip to content

Conversation

@vonzshik
Copy link
Contributor

No description provided.

@vonzshik vonzshik requested a review from roji as a code owner December 23, 2025 08:09
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Just checking: don't we need to pass --blame-hang (and maybe --blame, --blame-crash) for dotnet test above in order to produce these files?


- name: Upload Hang Dumps
uses: actions/upload-artifact@v6
if: always()
Copy link
Member

Choose a reason for hiding this comment

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

Is the if necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's kinda funny. Google's LLM said that yes, it's required. Checking again it now says that it should be failure() instead of always(). But anyway yes, we do have to have that if as otherwise we'll skip this step.

dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.DependencyInjection.Tests --logger "GitHubActions;report-warnings=false"
shell: bash

- name: Upload Hang Dumps
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Upload Hang Dumps
- name: Upload Test Hang Dumps

@vonzshik
Copy link
Contributor Author

Just checking: don't we need to pass --blame-hang (and maybe --blame, --blame-crash) for dotnet test above in order to produce these files?

Already have it.

- name: Test
run: |
dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.Tests --logger "GitHubActions;report-warnings=false" --blame-hang-timeout 30s

We can also add --blame-crash, though I do not think we crashed ever.

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.

3 participants