Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

ProjectManager.getAllFiles() order not consistent between OSs #9448

Closed
marcelgerber opened this issue Oct 5, 2014 · 8 comments
Closed

ProjectManager.getAllFiles() order not consistent between OSs #9448

marcelgerber opened this issue Oct 5, 2014 · 8 comments

Comments

@marcelgerber
Copy link
Contributor

When I investigated https://groups.google.com/d/msg/brackets-dev/sLouD1zl6Nk/4jQ6-ErFo2cJ (Linux-only issue), I found out that ProjectManager.getAllFiles() (probably even FileSystemEntry.visit()) has no consistent sort order between operating systems.
Most times, that's not much of an issue, but apparently CSSInlineEditor (and its tests) assumes there's a specific order.

Example:
Calling getAllFiles() on src/test/spec/CSSInlineEdit-test-files/css/ results in ["test.css", "test2.css"] on Win, but ["test2.css", "test.css"] on Ubuntu.

cc @ingorichter

@Mark-Simulacrum
Copy link
Contributor

Seems to be related to #8889, at least in the common test failures.

@marcelgerber
Copy link
Contributor Author

Well, this issue is about the CSSInlineEdit tests (which were BTW added this release), while #8889 is about the failing QuickOpen tests

@Mark-Simulacrum
Copy link
Contributor

The link to the issue on google groups (https://groups.google.com/d/msg/brackets-dev/sLouD1zl6Nk/4jQ6-ErFo2cJ) referred to Quick Open tests:

Andrew MacKenzie says:

3/3 Quick Open tests fail. Quick open works fine, so it seems to be a test issue. On Laptop 2, I run with an extended desktop spanning an external monitor and the built in display. They are both 900 high so the combined resolution is 3400x900. This confuses the quick open tests, I think as the errors I get are like this

@marcelgerber
Copy link
Contributor Author

But I investigated the failing CSSInlineEdit test ;)
So after all, I wouldn't call this a dupe. You're correct, the failing QuickOpen tests are already reported.

@Mark-Simulacrum
Copy link
Contributor

Ah, I see. Sorry for misunderstanding.

@peterflynn
Copy link
Member

The getAllFiles() API isn't intended to return a sorted list -- the order is actually nondeterministic, and can vary even on the same machine depending on IO timing. We discussed this today and agreed the API should stay unsorted (I'll see about tweaking the docs to make that more clear). Closing this bug for that reason.

But we should definitely fix any unit tests or features that incorrectly assume things about the list order. It looks like the only known case of that is the CSS Quick Edit unit tests -- which I think actually boils down to the CSS Quick Edit feature itself. I'm planning to address that as part of #8830, so that one is already tracked. If you find other cases of bad assumptions being made about this API, please feel free to file bugs on those cases.

@marcelgerber
Copy link
Contributor Author

Nice, the tests are indeed fixed.

@peterflynn
Copy link
Member

The CSS Quick Edit unit tests should be fixed by #9614, and I've included the documentation tweak in #9705 to clarify that the list is unsorted.

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

No branches or pull requests

3 participants