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

[5.0.3] Fix event counter issues and add test patterns for event source testing #23826

Merged
merged 2 commits into from
Jan 13, 2021

Conversation

ajcvickers
Copy link
Contributor

Fixes #23630

Description

Three event counters don't get updated when the async API is used (as opposed to the sync one). An additional event counter is incorrectly updated, and so show wrong results.

Customer Impact

Our newly-introduced event counters show incorrect data which does not take into account async calls.

How found

Customer reported an issue on 5.0.0 with one counter, the rest discovered via due diligence.

Test coverage

Test coverage added after investigation and discussion of the best way to do it. We settled on using reflection to access the counters and then configuring the tests to not run in parallel. These tests are both for detecting regressions, and so that going forward we can test new event counters to avoid mistakes like this in the future.

Regression?

No, event counters are new in 5.0.0.

Risk

Very low, add missing counter updates which are already in place and working for the sync versions, and move the location of another counter update. Only affects the new event counters feature.

…ce testing

Fixes #23630

**Description**

Three event counters don't get updated when the async API is used (as opposed to the sync one). An additional event counter is incorrectly updated, and so show wrong results.

**Customer Impact**

Our newly-introduced event counters show incorrect data which does not take into account async calls.

**How found**

Customer reported an issue on 5.0.0 with one counter, the rest discovered via due diligence.

**Test coverage**

Test coverage added after investigation and discussion of the best way to do it. We settled on using reflection to access the counters and then configuring the tests to not run in parallel. These tests are both for detecting regressions, and so that going forward we can test new event counters to avoid mistakes like this in the future.

**Regression?**

No, event counters are new in 5.0.0.

**Risk**

Very low, add missing counter updates which are already in place and working for the sync versions, and move the location of another counter update. Only affects the new event counters feature.
@ajcvickers ajcvickers requested a review from a team January 6, 2021 22:28
@ajcvickers ajcvickers added this to the 5.0.x milestone Jan 6, 2021
test/EFCore.Tests/EventSourceTest.cs Outdated Show resolved Hide resolved
test/EFCore.Tests/EventSourceTest.cs Outdated Show resolved Hide resolved
test/EFCore.Tests/EventSourceTest.cs Outdated Show resolved Hide resolved
test/EFCore.Tests/EventSourceTest.cs Outdated Show resolved Hide resolved
test/EFCore.Tests/EventSourceTest.cs Outdated Show resolved Hide resolved
@leecow leecow modified the milestones: 5.0.x, 5.0.3 Jan 12, 2021
@wtgodbe wtgodbe merged commit a37330f into release/5.0 Jan 13, 2021
@wtgodbe wtgodbe deleted the EventPlanner0106 branch January 13, 2021 17:16
@ajcvickers ajcvickers removed this from the 5.0.3 milestone Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants