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

Move remaning compile-fail tests that are rejected by the parser to pars... #22118

Merged
merged 2 commits into from
Feb 19, 2015

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 9, 2015

After making rustc fail on errors at a stop point, like -Z parse-only, in #22117, the files in this PR also fail during the parse stage and should be moved as well. Sorry for spliting this move up in two PRs.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Hm I don't actually think we're running any parse-fail tests at all! Looking at the most recent output for a successful test run there are no mentions of any parse-fail tests. I think that this change was only meant for re-running compiletest, the actual crawling of a directory comes from the --src-base argument and the logic may need to be tweaked to have "two source bases" for this.

Can you update this PR as well to be sure to run all the just-moved parse-fail tests and make sure that make check runs all tests?

@Manishearth
Copy link
Member

You might want to add Manishearth@19d8d3e to this PR too.

@fhahn
Copy link
Contributor Author

fhahn commented Feb 9, 2015

@alexcrichton damn, sorry! Should I modify compiletest.rs to accept more than a single path as --src-base or should I add a separate make target for parse-fail?

@alexcrichton
Copy link
Member

Whatever is easiest is fine by me, I'd probably go with multiple --src-base arguments as I think getopts already handles it. Also no worries, don't sweat it!

@fhahn
Copy link
Contributor Author

fhahn commented Feb 11, 2015

I've opted to add another pfail target, because I was not sure how to add multiple --src-base arguments in the Makefile in a generic way. Also note that at the moment, I am using the CompileFail mode of compiletest, so running the pfail tests prints [compile-fail] parse-fail/..... This may seem confusing, should I change that?

However I am not sure which part of the Makefile is responsible for copying the test files to the test directory. Any idea what I could have missed or pointers where I could start looking?

@alexcrichton
Copy link
Member

When you say copying test files, what are you referring to? I think your changes here should be sufficient for running the tests.

@fhahn fhahn force-pushed the separate-parse-fail-2 branch 2 times, most recently from 6eedfbb to 3845c39 Compare February 13, 2015 08:49
@fhahn
Copy link
Contributor Author

fhahn commented Feb 13, 2015

I got it wrong, I just had to create the ´test/parse-fail´ directory in the configure script. It should run the parse-fail tests now. But compiletest still prints [compile-fail] parse-fail/...., because I am reusing this code. Could this be somehow confusing?

@fhahn
Copy link
Contributor Author

fhahn commented Feb 13, 2015

@Manishearth the test case you reference fails during macro expansion (or later, I am not sure), because the include! has to be expanded first. So I think adding it to parse-fail would not be such a good idea, because I would like to add the -Z parse-only option to all parse-fail tests and the LALR parser does not handle macro expansion (although we could include some extra code to also parse files included with include!)

@brson
Copy link
Contributor

brson commented Feb 13, 2015

@fhahn I would prefer if you could add a ParseFail mode to compiletest if it's easy to do.

@fhahn
Copy link
Contributor Author

fhahn commented Feb 13, 2015

@brson I've updated the code.

$(Q)$(CFG_PYTHON) $(S)src/etc/check-summary.py tmp/*.log

# Only check the 'reference' tests: rpass/cfail/rfail/rmake.
check-ref: cleantestlibs cleantmptestlogs check-stage2-rpass check-stage2-rpass-valgrind \
check-stage2-rfail check-stage2-cfail check-stage2-rmake
check-stage2-rfail check-stage2-cfail check-stage2-pfail ccheck-stage2-rmake
Copy link
Member

Choose a reason for hiding this comment

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

ccheck typo

Copy link
Member

Choose a reason for hiding this comment

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

(ping typo)

@alexcrichton
Copy link
Member

Thanks @fhahn, looks good to me! Just a small typo and otherwise r=me.

Also, just to confirm, but you see the output of the tests in make check?

@fhahn
Copy link
Contributor Author

fhahn commented Feb 16, 2015

@alexcrichton I have fixed the type and now parse-fail and compile-fail tests show up when running make check

@alexcrichton
Copy link
Member

(I think the typo is still there)

@fhahn fhahn force-pushed the separate-parse-fail-2 branch from 33656fa to 6824f13 Compare February 17, 2015 00:01
@fhahn
Copy link
Contributor Author

fhahn commented Feb 17, 2015

Argh yes, I somehow forgot to push the updated commit. Sorry for that!

@alexcrichton
Copy link
Member

@bors: r+ 6824f13

(I'll fix the typo in the upcoming rollup)

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 19, 2015
After making `rustc` fail on errors at a stop point, like `-Z parse-only`, in rust-lang#22117, the files in this PR also fail during the parse stage and should be moved as well. Sorry for spliting this move up in two PRs.
@bors bors merged commit 6824f13 into rust-lang:master Feb 19, 2015
@fhahn fhahn deleted the separate-parse-fail-2 branch February 20, 2015 23:34
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.

6 participants