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

Expand benchmarks to include all tests #216

Closed
L-as opened this issue Jan 28, 2022 · 6 comments · Fixed by #276
Closed

Expand benchmarks to include all tests #216

L-as opened this issue Jan 28, 2022 · 6 comments · Fixed by #276
Assignees
Labels

Comments

@L-as
Copy link
Member

L-as commented Jan 28, 2022

We can simply change the testing implementation. See examples/Utils.hs.

@TotallyNotChase
Copy link
Collaborator

Not all the tests should be (or can be) benchmarked though, right? We should probably include a bench + test utility but also allow raw tests.

@L-as
Copy link
Member Author

L-as commented Jan 28, 2022

I think they all can be benchmarked.

@srid
Copy link
Member

srid commented Jan 28, 2022

Some tests evaluate more than one program. Might require some reorg here (for the foo.bar.qux hierarchical naming to make sense).

@srid srid added the testing label Jan 28, 2022
@L-as
Copy link
Member Author

L-as commented Jan 28, 2022

Well it's always either one or two. We could call the benches "name:1" and "name:2" when there are two.

@srid srid self-assigned this Feb 3, 2022
@srid
Copy link
Member

srid commented Feb 3, 2022

We can pull in the deterministic benchmarks into tests via golden testing (which also could cover the printTerm stuff, in principle).

It also solves the problem of performance regressions being overlooked (as a result of no one bothering to consistently check the Hercules CI effects output for benchmark diff), because now the CI would fail and the benchmark goldens will have to be reviewed/updated by the PR author. cf. #123

@srid srid linked a pull request Feb 16, 2022 that will close this issue
5 tasks
@L-as L-as closed this as completed in #276 Feb 16, 2022
@srid
Copy link
Member

srid commented Feb 17, 2022

This is only partly done; and will be completed as part of #302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants