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

rustc: gensym the module names for --test to avoid introducing names. #15964

Merged
merged 3 commits into from
Aug 9, 2014

Conversation

huonw
Copy link
Member

@huonw huonw commented Jul 25, 2014

This requires avoiding quote_...! for constructing the parts of the
__test module, since that stringifies and reinterns the idents, losing
the special gensym'd nature of them. (#15962.)

@sfackler
Copy link
Member

Ah, that's what the problem was! (Also this doesn't compile: https://travis-ci.org/rust-lang/rust/jobs/30801909)

@huonw
Copy link
Member Author

huonw commented Jul 25, 2014

Err, yes, I always compile the patch I actually commit. always

(Will fix tonight.)

…ccessible names.

This requires avoiding `quote_...!` for constructing the parts of the
__test module, since that stringifies and reinterns the idents, losing
the special gensym'd nature of them. (rust-lang#15962.)
@huonw
Copy link
Member Author

huonw commented Aug 7, 2014

@alexcrichton this is failing due to two test using the __test module directly:

---- [run-pass] run-pass/core-run-destroy.rs stdout ----

    error: compilation failed!
    status: exit code: 101
    command: x86_64-apple-darwin/stage2/bin/rustc /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/core-run-destroy.rs -L x86_64-apple-darwin/test/run-pass --target=x86_64-apple-darwin -L x86_64-apple-darwin/test/run-pass/core-run-destroy.stage2-x86_64-apple-darwin.libaux -C prefer-dynamic -o x86_64-apple-darwin/test/run-pass/core-run-destroy.stage2-x86_64-apple-darwin --cfg rtopt --cfg debug -O -L x86_64-apple-darwin/rt --test
    stdout:
    ------------------------------------------

    ------------------------------------------
    stderr:
    ------------------------------------------
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/core-run-destroy.rs:58:50: 58:62 error: failed to resolve. Use of undeclared module `__test`
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/core-run-destroy.rs:58     green::start(argc, argv, rustuv::event_loop, __test::main)
                                                                                                                                                             ^~~~~~~~~~~~
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/core-run-destroy.rs:58:50: 58:62 error: unresolved name `__test::main`.
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/core-run-destroy.rs:58     green::start(argc, argv, rustuv::event_loop, __test::main)
                                                                                                                                                             ^~~~~~~~~~~~
    error: aborting due to 2 previous errors

    ------------------------------------------

    task '[run-pass] run-pass/core-run-destroy.rs' failed at 'explicit failure', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/compiletest/runtest.rs:1321


---- [run-pass] run-pass/tcp-connect-timeouts.rs stdout ----

    error: compilation failed!
    status: exit code: 101
    command: x86_64-apple-darwin/stage2/bin/rustc /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/tcp-connect-timeouts.rs -L x86_64-apple-darwin/test/run-pass --target=x86_64-apple-darwin -L x86_64-apple-darwin/test/run-pass/tcp-connect-timeouts.stage2-x86_64-apple-darwin.libaux -C prefer-dynamic -o x86_64-apple-darwin/test/run-pass/tcp-connect-timeouts.stage2-x86_64-apple-darwin --cfg rtopt --cfg debug -O -L x86_64-apple-darwin/rt --test
    stdout:
    ------------------------------------------

    ------------------------------------------
    stderr:
    ------------------------------------------
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/tcp-connect-timeouts.rs:28:50: 28:62 error: failed to resolve. Use of undeclared module `__test`
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/tcp-connect-timeouts.rs:28     green::start(argc, argv, rustuv::event_loop, __test::main)
                                                                                                                                                                 ^~~~~~~~~~~~
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/tcp-connect-timeouts.rs:28:50: 28:62 error: unresolved name `__test::main`.
    /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/run-pass/tcp-connect-timeouts.rs:28     green::start(argc, argv, rustuv::event_loop, __test::main)
                                                                                                                                                                 ^~~~~~~~~~~~
    error: aborting due to 2 previous errors

    ------------------------------------------

    task '[run-pass] run-pass/tcp-connect-timeouts.rs' failed at 'explicit failure', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/compiletest/runtest.rs:1321

Thoughts? (I take it these tests are explicitly designed to be run under the green scheduler?)

@alexcrichton
Copy link
Member

Hm, that's interesting! The stdtest binary is also intended to run under the green scheduler by default (and rustuv I think).

That being said, the way all these binaries run the test harness under libgreen is a gross hack (as you're discovering).

Other possibilities might include:

  • #![please_test_my_crate_with_librustuv]
    • This could tweak injecting extern crate native vs extern crate green.
    • This could pass a boolean flag to the main function of the test runner whether it should be native or green. The test runner would then spin up its own green pool.
  • PLEASE_TEST_MY_CRATE_WITH_LIBRUSTUV=1 ./my-test-binary
    • This would work by the test runner spinning up its own green pool.
  • ./my-test-binary --please-test-my-crate-with-librustuv
  • #[call_top_level_function_with_relevant_test_arguments(my_main)]
    • Basically instead of calling test::test_main_static, the function __test::main would call my_main with the os arguments and TESTS static.
  • #![disable_test_hygiene]
    • The hacks today would continue to work as-is.
  • #![no_inner_test_mod]
    • This would inject the TESTS variable at the top level of a crate instead of in a hidden module. That way custom setup could be done before it's passed off to the test runner.

I'm not a fan of an environment variable or a command line flag because stdtest cannot succeed unless it's run with librustuv as the backend. This is kinda just a smattering of ideas, none of them make me feel comfortable really. I suppose the "sanest" one is #![disable_test_hygiene], but that is quite unfortunate!

@huonw
Copy link
Member Author

huonw commented Aug 8, 2014

I implemented a variation on #[disable_test_hygiene]: #![reexport_test_harness_main = "test_main"] will create a use test_main = __test::main (with the right gensyms) at the top of the crate, so one has access to the entrypoint via test_main (or whatever other symbol one choice).

r?

@@ -0,0 +1,9 @@
#[test]
Copy link
Member

Choose a reason for hiding this comment

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

linting may require that this need a license

implementation details.

(Mainly to avoid accessing the secret internal test module symbol name.)
default entrypoint of the --test binary.

This allows one to, e.g., run tests under libgreen by starting it
manually, passing in the test entrypoint.
bors added a commit that referenced this pull request Aug 9, 2014
This requires avoiding `quote_...!` for constructing the parts of the
__test module, since that stringifies and reinterns the idents, losing
the special gensym'd nature of them. (#15962.)
@bors bors closed this Aug 9, 2014
@bors bors merged commit edc9191 into rust-lang:master Aug 9, 2014
@huonw huonw deleted the gensym-test branch December 4, 2014 02:04
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 27, 2023
minor: Bump rustc deps and chalk

This finally upgrades `ra-ap-rustc_parse_format` (even though it's probably a no-op?).
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.

4 participants