move extra::test to libtest

[liigo]

I don't think extra is a good/meaningful name for a library. libextra should disappear, and we move all of its sub modules out of it. This PR is just one of that steps: move extra::test to libtest.

I didn't add libtest to doc index, because it's an internal library currently.

Update:

All comments addressed. All tests passed. Rebased and squashed.

[huonw]

cc #8784.

[huonw]

This is the problem here: it is making

mod __something {
     use test;

     // ... things using `test::foo()`
}

but libtest has no module test. One fix would be to add a module to libtest/lib.rs that reexports the relevant functions, e.g.

mod libtest_satisfy_the_test_runner { 
    pub use {test_main_static, 
             TestDescAndFn,
             TestDesc, StaticTestName,
             StaticTestFn, StaticBenchFn};
}

(Change the let test_crate_id = token::str_to_ident("test"); to "libtest_satisfy_the_test_runner".)

Hopefully someone has a better idea because that solution is really ugly. :(

[alexcrichton]

This is why libstd has mod std inside of it, I think this just means that libtest needs to have mod test inside of it (for the same hack-ish reasons)

[liigo]

@huonw @alexcrichton Thank you, I'll try.

[alexcrichton]

Stylistically instead of

mod tests {
    extern crate test;
    use self::test::BenchHarness;
}

I prefer

mod tests {
    extern crate test;

    #[bench] fn bench(bh: &mut test::BenchHarness) { /* ... */ }
}

Could you update code examples and such to this?

[huonw]

@alexcrichton I disagree: BenchHarness is long enough as it is, and, anyway, the convention from the style guide is to import types unqualified.

[alexcrichton]

Ah I suppose so, I guess I was just a little rattled at all the use self::test::BenchHarness immediately after extern crate test

[liigo]

@huonw @alexcrichton Why we name BenchHarness so long? I would like to rename it to Bench in another PR. Any idea?

[alexcrichton]

Let's not rename BenchHarness as part of this PR.

[alexcrichton]

It also looks like compiletest should lose its extra dependency and pick up its test dependency.

[liigo]

I have updated this PR, all comments addressed. Ready to review and merge.
There is a note: in PR's description also. @alexcrichton @huonw ?

[alexcrichton]

We shouldn't be including chunks of commented out code, if it's no longer necessary it should be deleted.

[alexcrichton]

I also think this logic needs to be preserved, this is the purpose of the 'mod test' in `libtest/lib.rs'

[huonw]

We could just let libtest manage its own test scope, i.e. only insert extern crate test when we're not in test, otherwise, if we are in test, do nothing.

[liigo]

OK, I'll restore the commented out code in rustc, and add mod test to libtest, come back later. Though I don't know why we need do these special cases both in rustc and libtest. A simple extern crate test (in the current commit) just make it works (all tests passed). @alexcrichton @huonw

[alexcrichton]

If you inject extern crate test then you get this duality where libtest is being tested with another version of libtest. This causes all sorts of resolve pains when you want to start using one or the other. I think that it may be ok, but I would rather bootstrap libtest with itself if possible.

[huonw]

(It also leads to the weirdest error messages "expected test::Foo but found test::Foo".)

[liigo]

@huonw @alexcrichton I've updated this PR, added mod test to libtest. Please take another review. Thank you!