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.

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

CLI path directory expansion should be case-insensitive #3599

Closed
4 tasks done
stevenvachon opened this issue Dec 6, 2018 · 11 comments
Closed
4 tasks done

CLI path directory expansion should be case-insensitive #3599

stevenvachon opened this issue Dec 6, 2018 · 11 comments
Labels
area: usability concerning user experience or interface status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior type: discussion debates, philosophy, navel-gazing, etc.

Comments

@stevenvachon
Copy link

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend avoiding the use of globally installed Mocha.

Description

The directory expansion of files should result in a case-insensitively sorted list.

Steps to Reproduce

Run mocha test/ in a project with test/a-tests.js and test/B-tests.js.

Expected behavior: "a" suite runs before "B" suite

Actual behavior: "B" suite runs before "a" suite

Reproduces how often: always

Versions

  • The output of mocha --version and node node_modules/.bin/mocha --version: 5.2.0
  • The output of node --version: 10.14.1
  • The version and architecture of your operating system: macOS 10.14.1
  • Your shell (bash, zsh, PowerShell, cmd, etc.): bash
  • Your browser and version (if running browser tests):
  • Any other third party Mocha related modules (with versions):
  • The code transpiler being used:

Additional Information

@plroebuck
Copy link
Contributor

Why? We support case-sensitive filesystems too. You could just name your tests so they would alphabetize in ASCII order.

@stevenvachon
Copy link
Author

It's unexpected behaviour, at least compared to glob.

@boneskull boneskull added status: accepting prs Mocha can use your help with this one! area: usability concerning user experience or interface good first issue new contributors should look here! labels Jan 2, 2019
@boneskull
Copy link
Member

When given a directory, Mocha uses fs.readdir(), which returns the files in this order.

If you need a specific order, consider using --file.

Maybe it'd be worthwhile to use glob under the hood instead of fs.readdir() so the ordering is consistent.

The function that handles this is in lib/utils.js. Instead of fs.readdir() we could use glob.sync() to look for <dirpath>/*.{<extensions>} where dirpath in this case would be test, and extensions is the array of extensions to load/watch (by default this is ['js']), comma-separated. If --recursive is used, change the /* in the globspec to /**/*.

@plroebuck
Copy link
Contributor

plroebuck commented Jan 3, 2019

We don't set the nocase glob option, so this ordering was not intentional.
If glob's output is ordered differently on case-insensitive filesystems, sorting its output before return would be simpler (and faster) means of consistency, without surprising all the non-(FAT/NTFS/HFS+) users with non-alphabetical ordering.

@plroebuck
Copy link
Contributor

plroebuck commented Jan 3, 2019

Having looked further, no order is implied via fs.readdir. Different OS/filesystem combinations can give different results (via readdir(3)) which returns [directory] entries in traversal order which may or may not be alphabetical (e.g., on macOS, HFS+ uses lexical order and APFS uses filename hash order). glob uses fs.readdir under the covers, but not necessarily processing in a straight line.

See also:

@boneskull
Copy link
Member

@plroebuck Are you suggesting to enable sorting by default (i.e., --sort)?

@boneskull boneskull removed good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one! labels Jan 3, 2019
@boneskull
Copy link
Member

I don't think we should change this behavior, but happy to discuss. Mocha makes no guarantees about the order in which files are loaded if it is not operating on a globspec (this is undocumented). That said...

Changing the behavior to sort differently (whatever that looks like) will break tests. And not of the "dirty tests" variety either--users may have some setup hooks within the root suite, which is a perfectly valid use case. Other workarounds such as --file are available, but this still forces users to change stuff for a relatively flimsy reason.

@boneskull boneskull added the type: discussion debates, philosophy, navel-gazing, etc. label Jan 3, 2019
@stevenvachon
Copy link
Author

stevenvachon commented Jan 3, 2019

I'm not sure how macOS sorts files, but Finder.app and glob both sort how I'd described in "expected behaviour".

@plroebuck
Copy link
Contributor

@plroebuck Are you suggesting to enable sorting by default (i.e., --sort)?

No. Original suggestion was only sorting glob results (attempting to make them match those from fs.readdir which at the time I assumed were alphabetical).

@plroebuck
Copy link
Contributor

I'm not sure how macOS sorts files, but Finder.app and glob both sort how I'd described in "expected behaviour".

Each Finder.app window explicitly sorts its contents via selected column header (e.g., Name, Size, Date Modified, Kind). Many things can also affect sorting order (e.g., locale, comparison function); even Finder.app's sort order can be changed under the hood.

@plroebuck plroebuck added the status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior label Jan 5, 2019
@plroebuck
Copy link
Contributor

I updated the API documentation to specify no order should be assumed.
Using consistent file naming and --sort will provide alphabetical ordering if required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: usability concerning user experience or interface status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior type: discussion debates, philosophy, navel-gazing, etc.
Projects
None yet
Development

No branches or pull requests

3 participants