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.

Sign up for GitHub

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

Jump to bottom

Port our tests to assert_cmd #522

Closed
wants to merge 1 commit into from
Closed

Port our tests to assert_cmd #522

wants to merge 1 commit into from

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Jan 24, 2019

Encountering some hanging tests on my machine; not 100% sure what is going on yet.

@ashleygwilliams ashleygwilliams added the work in progress do not merge! label Jan 24, 2019
@danwilhelm
Copy link
Member

danwilhelm commented Jan 24, 2019 •
edited
Loading

@fitzgen It seems there are accidentally two fixture.lock()s in a row in test::it_can_find_a_webdriver_on_path. The second is blocking until the first is released, but it will never be released! This causes all subsequent tests to time out (perhaps since they are run in parallel by default?).

Also, I noticed your tests are using an old hard-coded bindgen version. You may consider updating this version, or get it from the lockfile as in build.rs.

@danwilhelm danwilhelm mentioned this pull request Jan 24, 2019
Merged
@fitzgen
Copy link
Member Author

fitzgen commented Jan 24, 2019

@danwilhelm d'oh! thanks for taking a look :-p

@fitzgen fitzgen force-pushed the use-assert-cmd-everywhere branch from 6d566d4 to 83ed220 Compare January 28, 2019 22:13
@fitzgen fitzgen force-pushed the use-assert-cmd-everywhere branch from 83ed220 to f0b462c Compare January 28, 2019 22:15
@fitzgen fitzgen changed the title WIP port our tests to assert_cmd Port our tests to assert_cmd Jan 28, 2019
@fitzgen
Copy link
Member Author

fitzgen commented Jan 28, 2019

Ok this is ready for review.

r? @ashleygwilliams or @alexcrichton

@fitzgen fitzgen removed the work in progress do not merge! label Jan 28, 2019
@ashleygwilliams ashleygwilliams added this to the 0.6.1 milestone Jan 29, 2019
@ashleygwilliams ashleygwilliams self-requested a review January 29, 2019 14:56
ashleygwilliams
ashleygwilliams approved these changes Jan 29, 2019
View reviewed changes
Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

this looks awesome- so excited to write tests now! i'll take this and run cargo fmt and fix the merge conflict.

@ashleygwilliams ashleygwilliams self-assigned this Jan 29, 2019
@ashleygwilliams ashleygwilliams mentioned this pull request Jan 29, 2019
Merged
@ashleygwilliams
Copy link
Member

closing in favor of #534

@fitzgen fitzgen added the TWiRaWA Nominate this PR for inclusion in the next issue of This Week in Rust and WebAssembly label Feb 12, 2019
@ashleygwilliams ashleygwilliams mentioned this pull request Feb 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashleygwilliams ashleygwilliams ashleygwilliams approved these changes

@alexcrichton alexcrichton alexcrichton approved these changes

Assignees

@ashleygwilliams ashleygwilliams

Labels
changelog - maintenance TWiRaWA Nominate this PR for inclusion in the next issue of This Week in Rust and WebAssembly
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@fitzgen @danwilhelm @ashleygwilliams @alexcrichton