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

Organizing benching code #116

Open
behnam opened this issue Aug 13, 2017 · 4 comments
Open

Organizing benching code #116

behnam opened this issue Aug 13, 2017 · 4 comments
Assignees
Labels
A: test Testing enhancement Enhancements to existing features

Comments

@behnam
Copy link
Member

behnam commented Aug 13, 2017

In unic-bidi (and going back to the source unicode-bidi crate), I had added bench_it feature to not run benching on every single test run.

In travis.yml, benching is only run once and on the nightly rustc.

I think we did this because #[bench] was not stable yet. I think it's got stabilized now. Is that so?

Also, it is common to have simple data-less benches inside the modules, like unit tests, and not as an integration thing. Should we do this, or not?

@CAD97
Copy link
Collaborator

CAD97 commented Aug 13, 2017

For code in the benches folder, the bench_it feature is pointless because it's only compiled and run when you use cargo bench.

Benchmarks are unstable (rust-lang/rust#29553) because extern crate test requires a feature gate.

Without another gating feature, you cannot test on stable if you have #![cfg_attr(test, feature(test))].

(Somewhat surprisingly, #[cfg(bench)] doesn't complain. But it also doesn't enable when you do cargo bench like #[cfg(test)] does when you do cargo bench. Something to propose if stdlib benchmarks start moving towards stabilization.)

Personally, I feel #[bench] tests should be confined to just being run with cargo bench. But definitely, if benchmarks are only run with cargo bench (which only looks in the benches/ directory), the feature is not needed.

@behnam
Copy link
Member Author

behnam commented Aug 13, 2017

As you said, I believe there's no plan to have cfg(bench), and benches are considered part of cfg(test).

And, along the same line, I think all files under benches/ are built and run with cargo test, as well. I believe that was the reason I had to gate my benches with a feature, to only run on nightly, otherwise they would break a stable cargo test.

@CAD97
Copy link
Collaborator

CAD97 commented Aug 13, 2017

Well, if it was before, it's not anymore. unic-char-range has a benches directory with benchmarks and no benchmarking feature.

@behnam
Copy link
Member Author

behnam commented Aug 13, 2017

Cool! If it works fine in rustc:1.17.0, we can drop the branch. I'll take care of it soon.

@behnam behnam self-assigned this Aug 13, 2017
@CAD97 CAD97 added the A: test Testing label Aug 14, 2017
@CAD97 CAD97 added this to the UNIC-0.7 milestone Sep 15, 2017
@behnam behnam modified the milestones: UNIC-0.7, UNIC-0.8 Feb 7, 2018
@behnam behnam removed this from the UNIC-0.8 milestone Jan 2, 2019
@behnam behnam added the enhancement Enhancements to existing features label Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: test Testing enhancement Enhancements to existing features
Projects
None yet
Development

No branches or pull requests

2 participants