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

Test for appropriate span on second custom derive #38607

Merged
merged 4 commits into from
Jan 10, 2017

Conversation

estebank
Copy link
Contributor

Adds test for and closes #36935.

@rust-highfive
Copy link
Collaborator

r? @brson

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

@estebank estebank force-pushed the test-for-36935 branch 2 times, most recently from 4cfe8af to dfb6ae6 Compare December 27, 2016 21:09
@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 1, 2017

📌 Commit dfb6ae6 has been approved by petrochenkov

@bors
Copy link
Contributor

bors commented Jan 1, 2017

⌛ Testing commit dfb6ae6 with merge 0fbeada...

@bors
Copy link
Contributor

bors commented Jan 1, 2017

💔 Test failed - status-appveyor

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 4, 2017
…nkov

Test for appropriate span on second custom derive

Adds test for and closes rust-lang#36935.
@estebank
Copy link
Contributor Author

estebank commented Jan 4, 2017

Has there been a change in how proc_macro is used in master?

error[E0463]: can't find crate for `proc_macro`
  --> C:\projects\rust\src/test\ui\custom-derive\auxiliary\plugin.rs:16:1
   |
16 | extern crate proc_macro;
   | ^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
error: aborting due to previous error

@alexcrichton
Copy link
Member

Currently the ui test suite is not configured to have librustc available when it runs, so proc-macro tests in the ui test suite aren't guaranteed to work. Rustbuild is nondeterministic in how it lays out steps, so that may explain why it worked locally.

If we really want to add this test as a ui test then the suite needs to be moved below like the other "fulldeps" suites.

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jan 4, 2017
…nkov

Test for appropriate span on second custom derive

Adds test for and closes rust-lang#36935.
@estebank
Copy link
Contributor Author

estebank commented Jan 8, 2017

@alexcrichton I'm trying to figure out why I needed to remove the ui assertion from the test_with_a_target test for the test suite to run successfully, and what are the consequences from me doing so.

@alexcrichton
Copy link
Member

@estebank no worries that's ok. You can actually change the assertion to testing that it's not present instead of testing that it's present.

That basically means that we only run the tests for hosts, not targets (which is correct)

@alexcrichton
Copy link
Member

r=me with that change

@estebank
Copy link
Contributor Author

estebank commented Jan 9, 2017

@alexcrichton, done. I also removed two duplicated asserts while I was at it.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 9, 2017

📌 Commit d8b3a64 has been approved by alexcrichton

frewsxcv pushed a commit to frewsxcv/rust that referenced this pull request Jan 9, 2017
@bors
Copy link
Contributor

bors commented Jan 9, 2017

⌛ Testing commit d8b3a64 with merge db073ca...

@bors
Copy link
Contributor

bors commented Jan 9, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member

alexcrichton commented Jan 9, 2017 via email

sanxiyn added a commit to sanxiyn/rust that referenced this pull request Jan 10, 2017
…hton

Test for appropriate span on second custom derive

Adds test for and closes rust-lang#36935.
bors added a commit that referenced this pull request Jan 10, 2017
Rollup of 11 pull requests

- Successful merges: #38606, #38607, #38623, #38664, #38799, #38816, #38836, #38839, #38841, #38849, #38874
- Failed merges: #38845
@bors bors merged commit d8b3a64 into rust-lang:master Jan 10, 2017
@estebank estebank deleted the test-for-36935 branch November 9, 2023 05:27
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.

Second panicking custom derive has bad span
6 participants