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

feat: Composite generics support #70

Merged
merged 3 commits into from
Mar 18, 2023

Conversation

defaulterrr
Copy link
Contributor

Hey there! I noticed that my last PR #67 did not quite cover all possible cases with generic declarations so i followed up with some fixes. I've also implemented Go-based snapshot testing, hope you don't mind!

The general idea right now is to use go/printer package to render constraints references instead of manual traversal and string concatenation

Changelog:

  • Added support for composite generics (see new test cases for examples)
  • Extracted interface evaluation into a separate package (to unbloat the main.go file)
  • Implemented automatic Go based snapshot testing instead of manual Makefile

testCase := testCase

t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather do it sequentially or at least limit the concurrency to GOMAXPROCs based value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the Go documentation (go help testflag and go help build), t.Parallel() execution is automatically limited by GOMAXPROCS (as the default value). So, in our case (single package), the binary will already be limited by GOMAXPROCS

-parallel n
Allow parallel execution of test functions that call t.Parallel, and
fuzz targets that call t.Parallel when running the seed corpus.
The value of this flag is the maximum number of tests to run
simultaneously.
While fuzzing, the value of this flag is the maximum number of
subprocesses that may call the fuzz function simultaneously, regardless of
whether T.Parallel is called.
By default, -parallel is set to the value of GOMAXPROCS.
Setting -parallel to values higher than GOMAXPROCS may cause degraded
performance due to CPU contention, especially when fuzzing.
Note that -parallel only applies within a single test binary.
The 'go test' command may run tests for different packages
in parallel as well, according to the setting of the -p flag
(see 'go help build').

I can definitely change this to sequential if you're still having doubts about the CPU load during the execution, this will only change the execution time, not the result

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's leave it as is if it's already bound by GOMAXPROCS, yep my concern was CPU load especially if the number of tests grows.

@hexdigest hexdigest merged commit 228ba65 into gojuno:master Mar 18, 2023
@hexdigest
Copy link
Collaborator

Thanks, @defaulterrr

It's in v3.1.2 already

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.

2 participants