Skip to content

Avoid loading System.Threading.Tasks.Extensions when not needed #5694

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

Merged
merged 4 commits into from
Jun 9, 2025

Conversation

Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Jun 4, 2025

This works-around cases where binding redirects are not properly generated in consumer projects, either due to missing Microsoft.NET.Test.Sdk dependency, or due to reasons not yet known.

@Youssef1313 Youssef1313 force-pushed the dev/ygerges/system-threading-extensions branch from 59e05f2 to 2936222 Compare June 4, 2025 12:27
@Youssef1313
Copy link
Member Author

/backport to rel/3.9

Copy link
Contributor

github-actions bot commented Jun 4, 2025

Started backporting to rel/3.9: https://github.com/microsoft/testfx/actions/runs/15442261447

@Youssef1313
Copy link
Member Author

/backport to rel/3.9

Copy link
Contributor

github-actions bot commented Jun 4, 2025

Started backporting to rel/3.9: https://github.com/microsoft/testfx/actions/runs/15443366680

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2025

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 76.34%. Comparing base (35ac25e) to head (83f4e0b).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
...est.TestAdapter/Extensions/MethodInfoExtensions.cs 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5694      +/-   ##
==========================================
- Coverage   76.39%   76.34%   -0.05%     
==========================================
  Files         600      600              
  Lines       36743    36767      +24     
==========================================
+ Hits        28068    28069       +1     
- Misses       8675     8698      +23     
Flag Coverage Δ
Debug 76.34% <95.23%> (-0.05%) ⬇️
integration 76.34% <95.23%> (-0.05%) ⬇️
production 76.34% <95.23%> (-0.05%) ⬇️
unit 76.34% <95.23%> (-0.05%) ⬇️

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

Files with missing lines Coverage Δ
...ter/MSTest.TestAdapter/Execution/TestMethodInfo.cs 80.79% <100.00%> (+1.03%) ⬆️
...est.TestAdapter/Extensions/MethodInfoExtensions.cs 82.24% <93.33%> (-13.14%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jkotas
Copy link
Member

jkotas commented Jun 4, 2025

Do you need a test for the situation that this is trying to "fix"?

If the binding redirects are messed up, it typically leads to end-less stream of problems. This workaround is likely just kicking the can down the road.

@Youssef1313
Copy link
Member Author

Do you need a test for the situation that this is trying to "fix"?

Probably, I can try to create a scenario where this is broken.

If the binding redirects are messed up, it typically leads to end-less stream of problems. This workaround is likely just kicking the can down the road.

I definitely agree. Once the user for example tries to actually use ValueTask as a return type of a test method (which is extremely uncommon), they will hit the failure to load the assembly again. However, it's not yet clear what's causing this in the first place, and it's more likely to be a user error causing binding redirects to not be correctly generated. I'm only trying to reduce the noise when users have an error on their side that causes binding redirects to not be correctly generated.

@Youssef1313
Copy link
Member Author

I'll get to writing a test for this later as few important stuff are showing up.

@Youssef1313 Youssef1313 merged commit 45dec3b into main Jun 9, 2025
8 checks passed
@Youssef1313 Youssef1313 deleted the dev/ygerges/system-threading-extensions branch June 9, 2025 08:23
Youssef1313 added a commit that referenced this pull request Jun 9, 2025
…oussef1313 in #5694 (backport to rel/3.9) (#5695)

Co-authored-by: Youssef1313 <youssefvictor00@gmail.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.

6 participants