document the test infrastructure #47

nikomatsakis · 2018-02-07T14:48:26Z

This content is mostly taken from the COMPILER_TESTS.md in the compiler source tree, but I added some more notes.

mgattozzi · 2018-02-07T14:58:01Z

src/tests/adding.md

+file, typically a Rust source file. Test files have a particular
+structure:
+
+- They always begin with the copyright notice;


Maybe it would be useful to link the copyright notice here!

ugh. I just wish it didn't exist. But I suppose you are right.

@nikomatsakis I totally understand your pain

mre · 2018-02-07T18:05:12Z

src/tests/adding.md

+# Adding new tests
+
+**In general, we expect every PR that fixes a bug in rustc to come
+accompanied with a regression test of some kind.** This test should


Super nitpicky, but "accompanied by" is more common. 🤓
https://ell.stackexchange.com/a/109564

mark-i-m

Thanks! This is a great chapter :)

I left a bunch of comments. Most are nits and minor suggestions.

mark-i-m · 2018-02-07T15:42:35Z

src/tests/intro.md

+they mean. In some cases, the test suites are linked to parts of the manual
+that give more details.
+
+- [`ui`](ui.html) -- tests that check the exact stdout/stderr from compilation


Where is this file? How is this passing CI?

Also, would it be possible to create an Appendix and add this file to it? We could also add the ParamEnv mini-chapter from #45

link was wrong, fixed, not sure about the CI question tho

mark-i-m · 2018-02-07T15:45:54Z

src/tests/intro.md

+  generates valid Rust code from the AST
+- `debuginfo` -- tests that run in gdb or lldb and query the debug info
+- `codegen` -- tests that compile and then test the generated LLVM
+  code to make sure that optimizing we want are kicking in etc


"that the optimizations"

Also, nit: "kicking in, etc."

mark-i-m · 2018-02-07T15:46:49Z

src/tests/intro.md

+- `debuginfo` -- tests that run in gdb or lldb and query the debug info
+- `codegen` -- tests that compile and then test the generated LLVM
+  code to make sure that optimizing we want are kicking in etc
+- `mir-opt` -- tests that check parts of the generated MIR to make sure we are optimizing


nit: "optimizing, etc."

mark-i-m · 2018-02-07T15:48:13Z

src/tests/intro.md

+  when certain modifications are performed, we are able to reuse the
+  results from previous compilations.
+- `run-make` -- tests that basically just execute a `Makefile`; the ultimate in flexibility
+  but annoying as all get out to write.


This sentence does not make sense to me.

mark-i-m · 2018-02-07T15:48:53Z

src/tests/intro.md

+- `run-make` -- tests that basically just execute a `Makefile`; the ultimate in flexibility
+  but annoying as all get out to write.
+- `rustdoc` -- tests for rustdoc, making sure that the generated files contain
+  documentation for various entities etc


nit: "entities, etc."

mark-i-m · 2018-02-07T16:49:48Z

src/tests/adding.md

+...
+```
+
+Please see `ui/transmute/main.rs` and `.stderr` for a concrete usage example.


Can we link to github here?

mark-i-m · 2018-02-07T16:51:33Z

src/tests/adding.md

+
+The UI tests are intended to capture the compiler's complete output,
+so that we can test all aspects of the presentation. They work by
+compiling a file (e.g., `ui/hello_world/main.rs`), capturing the output,


Perhaps link to github here?

mark-i-m · 2018-02-07T16:52:14Z

src/tests/adding.md

+We have not traditionally had a lot of structure in the names of
+tests.  Moreover, for a long time, the rustc test runner did not
+support subdirectories (it now does), so test suites like
+`src/test/run-pass` have a huge mess of files in them. This is not


Perhaps link to github?

mark-i-m · 2018-02-07T16:54:42Z

src/tests/adding.md

@@ -0,0 +1,273 @@
+# Adding new tests


One thing I think would be nice to add to this chapter is some sort of heuristic for what type of tests to prefer. For example, should I prefer adding a parse-fail, compile-fail, or ui test, when any of them could test what I want to test?

mark-i-m · 2018-02-07T18:28:56Z

src/tests/running.md

+**Warning:** Note that bors only runs the tests with the full stage 2
+build; therefore, while the tests **usually** work fine with stage 1,
+there are some limitations. In particular, `./x.py test --stage 1`
+(ie.,


ehuss · 2018-02-07T18:18:58Z

src/tests/running.md

+**Warning:** Note that bors only runs the tests with the full stage 2
+build; therefore, while the tests **usually** work fine with stage 1,
+there are some limitations. In particular, `./x.py test --stage 1`
+(ie.,


This statement seems incomplete.

ehuss · 2018-02-07T18:24:47Z

src/tests/intro.md

+they mean. In some cases, the test suites are linked to parts of the manual
+that give more details.
+
+- [`ui`](ui.html) -- tests that check the exact stdout/stderr from compilation


I'm confused, what does this link to?

ehuss · 2018-02-07T18:47:04Z

Thanks @nikomatsakis, I find this very useful!

There also seems to be some additional information captured at https://brson.github.io/2017/07/10/how-rust-is-tested that may be useful. I was wondering if some of that can maybe be brought over? I can help if desired, but my perspective is from someone who knows very little about how things work (other than reading this), so I'm uncertain if I'm qualified.

mark-i-m · 2018-02-07T20:14:42Z

@ehuss I also really like that blog post. I think the main concern is how to not plagiarize it... One idea would be to just mention it as a further reference.

I can help if desired, but my perspective is from someone who knows very little about how things work (other than reading this), so I'm uncertain if I'm qualified.

Don't worry too much about this. I also don't know that much (except about macro parsing :P). Any help is appreciated!

mark-i-m · 2018-02-07T20:15:36Z

@nikomatsakis I thought of a couple of other things that we could mention here:

crater: what is it and why is it useful? What kinds of changes need a crater run?
tidy

QuietMisdreavus · 2018-02-07T20:24:20Z

Whenever this lands, i'd like to add a chapter talking about the things specific to the rustdoc tests. If you'd like, you can crib some initial content from the header of htmldocck.py. That's the best documentation for rustdoc tests at this point, and i'd like to bring it out of the test script.

mark-i-m · 2018-02-07T23:30:43Z

@QuietMisdreavus That's sounds great! Is there a reason why it must be done after this PR merges?

QuietMisdreavus · 2018-02-07T23:31:59Z

@mark-i-m Because i wanted to be the one to write it. 😜 I suppose i could make it a PR-to-the-PR, but i don't want to hold up this PR with extra content.

EDIT: Also, because this PR changes the folder structure, and i want to make a new file within that new folder.

mark-i-m · 2018-02-07T23:50:59Z

@QuietMisdreavus Got it :D

ehuss · 2018-02-08T01:13:27Z

Here are some other test-related things that might be useful to have. Perhaps they could be added as separate PRs? This guide seems to be very focused on the core of the compiler, so I'm uncertain how much you want it to talk about testing the standard library, and the tools and components that surround the compiler.

More discussion of the testing infrastructure. In particular, bors/homu, travis/appveyor, platform emulation, etc.
At least briefly mention the other tests that get run, such as:
- Unittests. In particular, how to run individual tests, roughly which libraries are automatically tested, etc. So far the guide seems to focus on just the tests in the test/ directory.
- Doctests. How to run the doc tests in the standard library. Other doc testing steps like linkchecker.
- distcheck
- tidy
- Mention that tests also get run for individual packages, such as bootstrap, cargo, rls, rustfmt, miri, clippy, cargotest.
crater/cargobomb?
Is fuzzing in active use by the team?
How does performance testing enter the picture?

phansch · 2018-02-10T11:40:26Z

src/tests/running.md

+(the same one you get with `#[test]`), so this command would wind up
+filtering for tests that include "issue-1234" in the name.
+
+Often, though, it's easier to just run the test by hand. More tests are 


I guess this should be 'Most tests'?

mark-i-m · 2018-02-16T22:50:03Z

@nikomatsakis Ping :)

nikomatsakis · 2018-02-17T14:26:32Z

Hi =) Yes, I meant to do this last week, but stuff got away from me. Gonna take a crack at addressing the feedback right now.

nikomatsakis · 2018-02-17T15:21:51Z

Things I did not do (I will add them to the main issue, they seem like good follow-up):

crater: what is it and why is it useful? What kinds of changes need a crater run?
- maybe talk about "perf runs" too
rustdoc tests
these things

nikomatsakis · 2018-02-17T15:26:36Z

I added those to the issue.

mark-i-m

@nikomatsakis LGTM! Thanks!

I had a quick question about when to add dependencies from crates.io, but I think we can merge and discuss that on the tracking issue.

mark-i-m · 2018-02-17T16:24:32Z

src/conventions.md

+It is allowed to use crates from crates.io, though external
+dependencies should not be added gratuitously. All such crates must
+have a suitably permissive license. There is an automatic check which
+inspects the Cargo metadata to ensure this.


Is there some sort of vetting that is done? Obviously, we don't want rustc's dependencies to explode and we don't want low-quality crates (or even malicious code).

There is not. There maybe should be.

See also https://internals.rust-lang.org/t/rustc-dependency-policy/6537

nikomatsakis requested a review from mark-i-m February 7, 2018 14:51

nikomatsakis force-pushed the test-infrastructure branch from beb2d89 to 679f76f Compare February 7, 2018 14:51

mgattozzi reviewed Feb 7, 2018

View reviewed changes

mre reviewed Feb 7, 2018

View reviewed changes

mark-i-m suggested changes Feb 7, 2018

View reviewed changes

ehuss reviewed Feb 7, 2018

View reviewed changes

phansch reviewed Feb 10, 2018

View reviewed changes

nikomatsakis mentioned this pull request Feb 12, 2018

master tracking issue #6

Closed

20 tasks

This was referenced Feb 13, 2018

"Using the compiler testing framework" #10

Closed

created compiletest.md #53

Merged

mark-i-m added the S-waiting-on-author Status: this PR is waiting for additional action by the OP label Feb 15, 2018

nikomatsakis added 7 commits February 17, 2018 09:27

document the test infrastructure

9d12cff

add some notes on coding conventions and link to copyright notice

8f9f5ef

add some more conventions

b195442

give some advice about which test suite to use

00ed1ea

complete sentence

6d64085

link to brson's blog post

361a4dd

fix typo

c9f7e5e

nikomatsakis force-pushed the test-infrastructure branch from 679f76f to c9f7e5e Compare February 17, 2018 15:21

nikomatsakis added 3 commits February 17, 2018 10:27

mention tidy

a0d1155

correct links (mdbook seems to want them relative to the book root)

a171e47

remove humorous footnote which is not supported

b52ba0d

mark-i-m approved these changes Feb 17, 2018

View reviewed changes

mark-i-m mentioned this pull request Feb 17, 2018

"Conventions used in the compiler" #12

Closed

8 tasks

mark-i-m merged commit c7c80ab into rust-lang:master Feb 17, 2018

mark-i-m mentioned this pull request Feb 17, 2018

CI using old cache of mdbook #58

Closed

document the test infrastructure #47

document the test infrastructure #47

Conversation

nikomatsakis commented Feb 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mark-i-m left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehuss commented Feb 7, 2018

mark-i-m commented Feb 7, 2018

mark-i-m commented Feb 7, 2018

QuietMisdreavus commented Feb 7, 2018

mark-i-m commented Feb 7, 2018

QuietMisdreavus commented Feb 7, 2018 • edited Loading

mark-i-m commented Feb 7, 2018

ehuss commented Feb 8, 2018

Choose a reason for hiding this comment

mark-i-m commented Feb 16, 2018

nikomatsakis commented Feb 17, 2018

nikomatsakis commented Feb 17, 2018

nikomatsakis commented Feb 17, 2018

mark-i-m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Feb 7, 2018 •

edited

Loading

mark-i-m left a comment •

edited

Loading

QuietMisdreavus commented Feb 7, 2018 •

edited

Loading