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

Cache the collected type information in getSourceLocation #518

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Feb 21, 2025

refs #517

Just having a go at the suggestion in #517 (comment)

I've tried a local build with a couple of my own projects in Visual Studio 2022 and it seems to still work and be a tad faster, but the test discovery only takes a few seconds for those anyway so it's a small difference.

@TheAngryByrd
Copy link
Contributor

If you want a an example to test. FsToolkit.ErrorHandling.Tests. Takes my machine about a little over ~20 to run with dotnet test but dotnet run is about 200ms.

YoloDev does have an upper bound for Expecto (>= 10.2.1 && < 11.0.0). Are you using another way or did you do a custom build of that as well?

@Numpsy
Copy link
Contributor Author

Numpsy commented Feb 21, 2025

Are you using another way or did you do a custom build of that as well?

I'm using a custom build (theres one at https://www.nuget.org/packages/Beardmouse.Expecto.Testing actually)

If you want a an example to test. FsToolkit.ErrorHandling.Tests.

With that, I get

Test summary: total: 1630, failed: 0, succeeded: 1622, skipped: 8, duration: 47.4s

with the current FsToolkit.ErrorHandling source, and

Test summary: total: 1630, failed: 0, succeeded: 1622, skipped: 8, duration: 3.7s

with it changed to use Expecto from this PR (so I'm not sure if the difference is just this change, or if it's picking up any other improvements in V11 as well)

For comparison, dotnet run takes barely over 1 second in either case

@farlee2121
Copy link
Collaborator

Dang. That is substantial.
The code looks good to me if you think it's ready

@Numpsy Numpsy marked this pull request as ready for review February 22, 2025 10:24
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.

3 participants