Skip to content

use Polyfill #6693

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
Merged
merged 12 commits into from
May 3, 2023
Merged

use Polyfill #6693

merged 12 commits into from
May 3, 2023

Conversation

SimonCropp
Copy link
Contributor

  • adds Polyfill to Akka.csproj
  • uses a ModuleInitializer (which polyfill adds support for) for test config in Akka.API.Tests.csproj
  • updated Verify.Xunit to use the simpler UseProjectRelativeDirectory api

@SimonCropp SimonCropp marked this pull request as draft April 28, 2023 06:33
@SimonCropp
Copy link
Contributor Author

need some help deciphering the build error. seems unrelated to my change?? https://dev.azure.com/dotnet/Akka.NET/_build/results?buildId=92267&view=logs&j=d83928d8-8bfe-51de-721e-9025f72ee732&t=21d77f07-2bef-56ac-91e9-8618e6456479

@Aaronontheweb
Copy link
Member

need some help deciphering the build error. seems unrelated to my change?? https://dev.azure.com/dotnet/Akka.NET/_build/results?buildId=92267&view=logs&j=d83928d8-8bfe-51de-721e-9025f72ee732&t=21d77f07-2bef-56ac-91e9-8618e6456479

We have some notoriously racy tests in our test suite - almost always they're caused by tests having tightly bounded execution times.

Akka.Persistence.Query.InMemory.Tests.InMemoryEventsByTagSpec.ReadJournal_live_query_EventsByTag_should_find_new_events

This is one of them. No worries - you're in good shape. You can mark this PR as ready for review when you're set @SimonCropp

@Aaronontheweb
Copy link
Member

The feature I'm adding to Akka.NET which depends on this is going to be brought into the main repo today I think - is this good for me to review?

@SimonCropp SimonCropp marked this pull request as ready for review May 3, 2023 12:49
@SimonCropp
Copy link
Contributor Author

@Aaronontheweb yeah. ready for review.

Aaronontheweb
Aaronontheweb approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

LGTM

src/core/Akka.API.Tests/Akka.API.Tests.csproj Outdated
@@ -29,6 +29,7 @@
<PackageReference Include="xunit" Version="$(XunitVersion)" />
<PackageReference Include="xunit.runner.visualstudio" Version="$(XunitVersion)" />
<PackageReference Include="PublicApiGenerator" Version="9.3.0" />
<PackageReference Include="Verify.Xunit" Version="19.14.0" />
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate reference - I'll remove it.

src/core/Akka.API.Tests/ModuleInit.cs
public static class ModuleInit
{
[ModuleInitializer]
public static void Init()
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

src/core/Akka/Akka.csproj
@@ -14,6 +14,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Include="Polyfill" Version="1.21.0" PrivateAssets="all" />
Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@Aaronontheweb
Copy link
Member

Looks great, thank you for this @SimonCropp

@Aaronontheweb Aaronontheweb added this to the 1.5.5 milestone May 3, 2023
@Aaronontheweb Aaronontheweb merged commit 95000a6 into akkadotnet:dev May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aaronontheweb Aaronontheweb Aaronontheweb approved these changes

Assignees
No one assigned
Labels
akka-actor api-change
Projects
None yet
Milestone
1.5.5
Development

Successfully merging this pull request may close these issues.
2 participants
@SimonCropp @Aaronontheweb