Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

fix: run tests on windows #127

Merged
merged 5 commits into from
Nov 24, 2020
Merged

fix: run tests on windows #127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
81c9e95
fix: run tests on windows
achingbrain Nov 13, 2020
82af2c8
Update cli.js
achingbrain Nov 24, 2020
e8606a8
linting
achingbrain Nov 24, 2020
f908462
replace backslashes with forward slashes
achingbrain Nov 24, 2020
96928de
Make the tests run on PR:s
voxpelli Nov 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
strategy:
matrix:
node_version: [10, 12, 14, 15]
os: [ubuntu-latest]
os: [ubuntu-latest, windows-latest]
steps:
- uses: actions/checkout@v1
- name: Use Node.js ${{ matrix.node_version }}
Expand Down
8 changes: 8 additions & 0 deletions cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ if (args.help || args._.length === 0) {
process.exit(1)
}

// windows leaves leading/trailing quotes on strings needed on unix to
// stop shells from doing path expansion, so strip them if present
args._ = args._.map(stripQuotes)

function extensions (arg) {
if (!arg) return undefined
const extensions = {}
Expand All @@ -73,6 +77,10 @@ function extensions (arg) {
return extensions
}

function stripQuotes (string) {
return string.replace(/(^'|")|('|"$)/g, '')
achingbrain marked this conversation as resolved.
Show resolved Hide resolved
}

check({
path: args._.shift(),
entries: args._,
Expand Down
4 changes: 2 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ async function resolveGlobbedPath (entries, cwd) {
expandDirectories: false
})

const paths = Object.keys(resolvedEntries.reduce((result, entry) => {
const paths = Object.values(resolvedEntries.reduce((result, entry) => {
// Globby yields unix-style paths.
const normalized = path.resolve(entry)

if (!result[normalized]) {
result[normalized] = true
result[normalized] = entry
}

return result
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes the normalized variable rather redundant, no? So we could just skip this entire reduce and just return resolvedEntries straight up?

Makes me wonder why its originally been added there at all 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I read it, the reason you resolve the path is to make all the requires absolute and then use the result hash to dedupe them?

Copy link
Collaborator

@voxpelli voxpelli Nov 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

That's true, good catch.

Because the code passes them to globby twice, the second time round nothing is matched because it doesn't understand forward slashes as path separators

I'm thinking that we maybe instead should ensure that whatever globby is given is always using the right slashes? Adding a entry.replace('\', '/') to

dependency-check/index.js

Lines 26 to 35 in ad715e5

async function resolveGlobbedPath (entries, cwd) {
if (typeof entries === 'string') entries = [entries]
debug('globby resolving', entries)
const resolvedEntries = await globby(entries, {
cwd,
absolute: true,
expandDirectories: false
})
What do you think? Then we will keep the OS-type path separators in the result of the method, which should be preferable considering the other file interactions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to work with replacing the slashes before passing the entries to globby, I've updated the PR.

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"lint": "standard",
"test-cli:custom-detective": "node cli.js test/ -e js:detective-cjs",
"test-cli:glob": "node cli.js 'test/**/*.js' --no-default-entries",
"test-cli:multi-glob": "node cli.js test/foo.js 'test/*.js' \"test/donkey/*.js\" --no-default-entries",
"test-cli:main-as-file": "node cli.js test/index.js",
"test-cli:simple": "node cli.js test/",
"test-cli": "run-p test-cli:*",
Expand Down