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
Member

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.
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
This pull request was closed.
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