Skip to content
This repository was archived by the owner on Nov 24, 2023. It is now read-only.

Fix test edge cases #51

Merged
merged 4 commits into from
Jan 18, 2018
Merged

Fix test edge cases #51

merged 4 commits into from
Jan 18, 2018

Conversation

killercup
Copy link
Member

Noticed this while adding test case for fun rust-lang/rust-clippy#2350

@killercup killercup requested a review from oli-obk January 17, 2018 20:45
@@ -143,7 +143,7 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P) -> Result<(), Box<Error>> {

let mut fixed = code.clone();

for sug in suggestions {
for sug in suggestions.into_iter().rev() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs a test

Copy link
Member Author

Choose a reason for hiding this comment

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

Only possible with an additional test that has a replacement that condenses n lines into m<n lines followed by ≥1 replacements.

…which I just added

@oli-obk
Copy link
Collaborator

oli-obk commented Jan 18, 2018

bors r+

bors bot added a commit that referenced this pull request Jan 18, 2018
51: Fix test edge cases r=oli-obk a=killercup

Noticed this while adding test case for ~~fun~~ rust-lang/rust-clippy#2350
Copy link
Collaborator

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Jan 18, 2018
51: Fix test edge cases r=oli-obk a=killercup

Noticed this while adding test case for ~~fun~~ rust-lang/rust-clippy#2350
@bors
Copy link

bors bot commented Jan 18, 2018

Build succeeded

@bors bors bot merged commit 9121d77 into master Jan 18, 2018
@oli-obk oli-obk deleted the fix-test-edge-cases branch January 18, 2018 11:16
@oli-obk
Copy link
Collaborator

oli-obk commented Jan 18, 2018

now we know that a missing review will even fail bors. Although the silent failure is suboptimal

@killercup
Copy link
Member Author

killercup commented Jan 18, 2018

Hah, thanks for testing ;)

What was the issue? That it couldn't merge the PR because Github rejected its request?

Do you want me to change the settings to only require bors but not a github review?

Edit

Although the silent failure is suboptimal

I see this in bors' dashboard FYI. Maybe bors is just too polite to leave such a ragequit comment on the PR?

{{:badmatch, {:error, :push}},
 [{BorsNG.GitHub, :push!, 3,
   [file: 'lib/github.ex', line: 47]},
  {BorsNG.Worker.Batcher, :complete_batch, 3,
   [file: 'lib/batcher.ex', line: 375]},
  {BorsNG.Worker.Batcher, :maybe_complete_batch, 1,
   [file: 'lib/batcher.ex', line: 362]},
  {BorsNG.Worker.Batcher, :handle_cast, 2,
   [file: 'lib/batcher.ex', line: 71]},
  {:gen_server, :try_dispatch, 4,
   [file: 'gen_server.erl', line: 616]},
  {:gen_server, :handle_msg, 6,
   [file: 'gen_server.erl', line: 686]},
  {:proc_lib, :init_p_do_apply, 3,
   [file: 'proc_lib.erl', line: 247]}]}

@oli-obk
Copy link
Collaborator

oli-obk commented Jan 18, 2018

Do you want me to change the settings to only require bors but not a github review?

nah, I'm a quick learner

@killercup
Copy link
Member Author

killercup commented Jan 18, 2018 via email

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants