Skip to content

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

Merged
Youssef1313 merged 4 commits into
mainfrom
dev/ygerges/system-threading-extensions
Jun 9, 2025
Merged

Avoid loading System.Threading.Tasks.Extensions when not needed#5694
Youssef1313 merged 4 commits into
mainfrom
dev/ygerges/system-threading-extensions

Conversation

@Youssef1313
Copy link
Copy Markdown
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
Copy Markdown
Member Author

/backport to rel/3.9

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2025

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

Comment thread src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs Outdated
Comment thread src/Adapter/MSTest.TestAdapter/Execution/TestMethodInfo.cs Outdated
@Youssef1313
Copy link
Copy Markdown
Member Author

/backport to rel/3.9

@github-actions
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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