Skip to content

Updates Dapr to 1.12 in GitHub actions itest#1185

Merged
halspang merged 7 commits into
dapr:masterfrom
JoshVanL:github-actions-update-dapr
Nov 29, 2023
Merged

Updates Dapr to 1.12 in GitHub actions itest#1185
halspang merged 7 commits into
dapr:masterfrom
JoshVanL:github-actions-update-dapr

Conversation

@JoshVanL
Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL JoshVanL requested review from a team as code owners November 13, 2023 13:48
Comment thread .github/workflows/itests.yml Outdated
Copy link
Copy Markdown
Contributor

@halspang halspang left a comment

Choose a reason for hiding this comment

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

Let's remove the commit ref as that's for updating the dapr version for a new feature (mid release).

Signed-off-by: joshvanl <me@joshvanl.dev>
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (abcbf4f) 66.53% compared to head (62c55ca) 66.53%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1185   +/-   ##
=======================================
  Coverage   66.53%   66.53%           
=======================================
  Files         171      171           
  Lines        5752     5752           
  Branches      626      626           
=======================================
  Hits         3827     3827           
  Misses       1776     1776           
  Partials      149      149           
Flag Coverage Δ
net6 66.52% <ø> (ø)
net7 66.52% <ø> (ø)
net8 66.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JoshVanL
Copy link
Copy Markdown
Contributor Author

@halspang Done, but looks like a real failure

@halspang
Copy link
Copy Markdown
Contributor

Yeah, that sure does. @RyanLettieri, can you take a look at this workflow failure?

@RyanLettieri
Copy link
Copy Markdown
Contributor

@JoshVanL The error looks like it is coming from an assert statement after we purge an instance and attempt to get the workflow instance back. The current statement we use to check for this is the following:
ex.InnerException.Message.Should().Contain("No such instance exists", $"Instance {instanceId} was not correctly purged");

Due to Should().Contain() being case-sensitive, the error is probably coming from a change in the capitalization of the error message. From the error logs:

Error: Dapr.E2E.Test.E2ETests.TestWorkflows: Expected string "Status(StatusCode="NotFound", Detail="unable to find workflow with the provided instance ID: testInstanceId%!(EXTRA *fmt.wrapError=Failed to fetch orchestration metadata: no such instance exists)")" to contain "No such instance exists" because Instance testInstanceId was not correctly purged.

It appears we check for a capital N on the error whereas the error in 1.12 seems to use a lower-case N. I'd recommend changing that line and re-running the test

Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL
Copy link
Copy Markdown
Contributor Author

Thanks @RyanLettieri! Really appreciate the detailed breakdown- I'm still getting used to dotnet so thanks for the help.

@halspang halspang merged commit b669585 into dapr:master Nov 29, 2023
divzi-p pushed a commit to divzi-p/dotnet-sdk that referenced this pull request Dec 10, 2024
* Updates Dapr to 1.12 in GitHub actions

Signed-off-by: joshvanl <me@joshvanl.dev>

* Remove commit ref from github actions

Signed-off-by: joshvanl <me@joshvanl.dev>

* Fix case sensitive error string match case

Signed-off-by: joshvanl <me@joshvanl.dev>

---------

Signed-off-by: joshvanl <me@joshvanl.dev>
Co-authored-by: halspang <70976921+halspang@users.noreply.github.com>
Signed-off-by: Divya Perumal <diperuma@microsoft.com>
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