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.

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

Updates Dapr to 1.12 in GitHub actions itest #1185

Merged
merged 7 commits into from
Nov 29, 2023

Conversation

JoshVanL
Copy link
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
Copy link
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>
Copy link

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
Contributor Author

@halspang Done, but looks like a real failure

@halspang
Copy link
Contributor

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

@RyanLettieri
Copy link
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
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
12 checks passed
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