pimterry / notes

:pencil: Simple delightful note taking, with more unix and less lock-in.
https://github.com/pimterry/notes
MIT License
1.24k stars 82 forks source link

BATS fails on "Show multiple files passed by pipe from find" #72

Closed primis closed 4 years ago

primis commented 4 years ago

Issue Summary

Commit 00f9e66cb336146d880a9eee07bb2e8cd90b756f which was merge request #71 to fix #38 fails BATS test for show multiple files passed by pipe from find.

The specific failure output is a reversed from expected output as shown in the Technical details section. This may be a failing of the test itself or a failing of the find code. However, manually running the test without BATS does not have this failing.

Steps to Reproduce

  1. Run BATS.

Technical details:

primis commented 4 years ago

To add: my bash version 4.4.12(1)-release is different from the Travis-CI version 4.3.48(1)-release. I am unsure if this has any bearing

pimterry commented 4 years ago

Hmm, this is very odd! I can reproduce it locally too, and it's now failing on Travis in the same way. Very odd, especially given that the PR you link above doesn't touch either this logic or the tests, just the completion file. Not sure what's changed to cause this.

When you say:

However, manually running the test without BATS does not have this failing.

what exactly do you mean? For me, running the test suite fails, as does running the one test file directly, or manually running the same commands by hand.

I can't get this test to pass at all now. As far as I can tell it's actually non-deterministic (I think the default sort order of find is not defined), but it's very odd that it's now consistently the 'wrong' order, whilst it was previously consistently the opposite.

In practice, the right solution is probably to either define a fixed order ourselves & sort the results, or make the test allow either order, I think there's arguments for both. I'd love to know why this has happened though, any ideas?

primis commented 4 years ago

By manually I mean that if I have two files, file1 and file2 and do a notes find | notes grep it always comes out in the expected order (1 before 2)

On August 29, 2019 11:53:31 AM EDT, Tim Perry notifications@github.com wrote:

Hmm, this is very odd! I can reproduce it locally too, and it's now failing on Travis in the same way. Very odd, especially given that the PR you link above doesn't touch either this logic or the tests, just the completion file. Not sure what's changed to cause this.

When you say:

However, manually running the test without BATS does not have this failing.

what exactly do you mean? For me, running the test suite fails, as does running the one test file directly, or manually running the same commands by hand.

I can't get this test to pass at all now. As far as I can tell it's actually non-deterministic (I think the default sort order of find is not defined), but it's very odd that it's now consistently the 'wrong' order, whilst it was previously consistently the opposite.

In practice, the right solution is probably to either define a fixed order ourselves & sort the results, or make the test allow either order, I think there's arguments for both. I'd love to know why this has happened though, any ideas?

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/pimterry/notes/issues/72#issuecomment-526249191

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.