Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Improve hover test_tooltip tests #1175

Merged
merged 6 commits into from
Dec 13, 2018

Conversation

alexheretic
Copy link
Member

@alexheretic alexheretic commented Dec 8, 2018

After the discussion in #1151 this change:

  • Splits test_tooltip into test_tooltip & test_tooltip_std. Only the latter will be ignored to avoid running upstream.
  • Remove the --cfg=enabled_tooltip_tests stuff, the ignored test can be run with cargo test -- --ignored.
  • Add string diff coloured output to tooltip tests.
Old/resolved

Windows Tests

I also tried to get to the bottom of the windows test failures. I managed to reproduce it on my local machine and traced the error to a panic without a backtrace from a rustfmt call:

let formatted = match rustfmt.format(object.clone(), config) {

error: unexpected close delimiter: `}`
 --> <stdin>:5:1
  |
5 | }
  | ^ unexpected close delimiter

This was quite painful to find but can be handled with std::panic::catch_unwind. I've added logging init to the tooltip tests. Hopefully we can get to the bottom of the still failing windows tests now the panic will not kill the test run.

@Xanewok
Copy link
Member

Xanewok commented Dec 8, 2018

The Windows CI test failure was caused by Racer - https://travis-ci.org/Xanewok/rls/jobs/465182472#L1336. Since it was changed some time ago to use byte offsets but still distinguished lines with lines() it led to a mismatch but can confirm that racer-rust/racer#1007 fixes the test now and autocompletion in Windows in general.

(However, right now toolstate is broken because of clippy so we'll need to wait for rust-lang/rust#56631 to land)

src/actions/hover.rs Outdated Show resolved Hide resolved
@alexheretic alexheretic force-pushed the split-tooltip-std branch 3 times, most recently from 163cc2e to ae0cdb3 Compare December 10, 2018 12:21
@alexheretic
Copy link
Member Author

I've also added string diffs for tooltip test failures which seem pretty nice.

@Xanewok
Copy link
Member

Xanewok commented Dec 10, 2018

@aloucks @alexheretic
I wonder - could we just vendor our own copy of std-based source files and not depend on rust-src component? Or the idea is to have it act as integration test with the actual component that users have?

Being able to run entire test suite inside the Rust CI would definitely simplify things.

@alexheretic
Copy link
Member Author

I'm not sure how that works, but being able to run the full test suite upstream would be great.

Copy link
Member

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

LGTM, I'd like to land this now modulo catch_unwind.

src/actions/hover.rs Outdated Show resolved Hide resolved
@Xanewok
Copy link
Member

Xanewok commented Dec 13, 2018

Thank you! Thankfully, for now Windows CI tests are fixed so I'll merge this and later update rustfmt asap when the upstream fix is merged.

@Xanewok Xanewok merged commit 9ddbb5a into rust-lang:master Dec 13, 2018
@alexheretic alexheretic deleted the split-tooltip-std branch December 13, 2018 16:57
bors added a commit to rust-lang/rust that referenced this pull request Dec 29, 2018
Update cargo, rls, miri

Update cargo, rls, miri

Added `rustc-workspace-hack` to miri so that it shares the same features for serde as other tools.

cc @alexcrichton

## cargo

25 commits in 2cf1f5dda2f7ed84e94c4d32f643e0f1f15352f0..0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4
2018-12-11 03:44:04 +0000 to 2018-12-19 14:45:14 +0000
- Remove Stale bot's configuration (rust-lang/cargo#6463)
- Add labels to issue templates (rust-lang/cargo#6464)
- Fix new man page links. (rust-lang/cargo#6459)
- Fix metabuild compile errors with --message-format=json. (rust-lang/cargo#6432)
- Support alt-registry names in [patch] table. (rust-lang/cargo#6456)
- Update the rustup URL (rust-lang/cargo#6455)
- New man pages. (rust-lang/cargo#6405)
- Reify the DepFingerprint type (rust-lang/cargo#6451)
- Extract Fingerprint::new (rust-lang/cargo#6449)
- Upgrade the metabuild to Rust 2018 (rust-lang/cargo#6448)
- Make edition comparing code consistent (rust-lang/cargo#6450)
- Document `name` and `authors` in [package] (rust-lang/cargo#6447)
- Travis: only use mdbook 0.1.7. (rust-lang/cargo#6443)
- Update git2-curl requirement from 0.8.1 to 0.9.0 (rust-lang/cargo#6439)
- Update git2 requirement from 0.7.5 to 0.8.0 (rust-lang/cargo#6438)
- Display errors when `cargo fix` fails. (rust-lang/cargo#6419)
- cargo fix: fix targets with shared sources. (rust-lang/cargo#6434)
- Fix panic-in-panic in tests. (rust-lang/cargo#6431)
- More Rust 2018 edition cleanups (rust-lang/cargo#6422)
- Cleanup some trait impls for SourceId (rust-lang/cargo#6429)
- Remove a nightly check from doc tests (rust-lang/cargo#6427)
- Replace CargoError with failure::Error (rust-lang/cargo#6425)
- Allow testsuite warnings in dev (rust-lang/cargo#6426)
- add `--dry-run` option to cargo update (rust-lang/cargo#6371)
- Migrate to some Rust 2018 idioms (rust-lang/cargo#6416)

## rls

16 commits in bd5b899afb05e14d33e210ede3da241ca1ca088f..6f5e4bba7b1586fca6e0ea7724cadb5683b2f308
2018-12-10 08:53:00 +0100 to 2018-12-21 17:11:08 +0100
- Update jsonrpc-core (rust-lang/rls#1206)
- Use `home_dir` from `home` crate (rust-lang/rls#1207)
- Update cargo. (rust-lang/rls#1204)
- Fix deprecated `trim_{left,right}` warnings (rust-lang/rls#1203)
- Respect ${CARGO,RUSTUP}_HOME for tooltip relative dirs (rust-lang/rls#1201)
- Separate tooltip tests that require Racer fallback (rust-lang/rls#1200)
- tests: Don't generate tooltip results in tests/fixtures (rust-lang/rls#1199)
- Overhaul fixture handling in tests (rust-lang/rls#1190)
- Don't return symbols with empty names (rust-lang/rls#1193)
- Don't check AppVeyor CI status for bors
- Properly infer full_docs (rust-lang/rls#1192)
- Update cargo (rust-lang/rls#1191)
- Improve hover test_tooltip tests (rust-lang/rls#1175)
- Fix unused warnings (rust-lang/rls#1185)
- Workaround rust-lang/rls#703 to prevent obscure failures due to sccache. (rust-lang/rls#1177)
- Disable travis cache (rust-lang/rls#1182)

## miri

14 commits in bccadeb..6c2fc6d
2018-12-08 11:07:22 +0100 to 2018-12-26 14:28:25 +0100
- use memory::check_bounds_ptr for offset check (rust-lang/miri#589)
- Fix comparing function pointers (rust-lang/miri#587)
- fix for infallible allocation (rust-lang/miri#586)
- fix test for latest nightly (rust-lang/miri#585)
- Treat ref-to-raw cast like a reborrow: do a special kind of retag (rust-lang/miri#572)
- Test cargo-miri on Windows (rust-lang/miri#578)
- Cargo miri tweaks and test that we can exclude tests (rust-lang/miri#580)
- Fix cargo miri test (rust-lang/miri#550)
- fix for latest nightly (rust-lang/miri#574)
- Add rustc-workspace-hack. (rust-lang/miri#575)
- use RUSTC_WRAPPER for the cargo hook (rust-lang/miri#573)
- do not auto-detect the targets in the sysroot, instead specify target manually through env var (rust-lang/miri#570)
- Cleanup: Avoid repeating signatures, get rid of to_bytes hack (rust-lang/miri#568)
- Support building and running with full MIR on foreign architectures, drop support for missing MIR (rust-lang/miri#566)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants