Skip to content

Updated tests to support Windows.#77

Open
DawMatt wants to merge 2 commits into
jmlue42:mainfrom
DawMatt:feature/cross-platform_tests
Open

Updated tests to support Windows.#77
DawMatt wants to merge 2 commits into
jmlue42:mainfrom
DawMatt:feature/cross-platform_tests

Conversation

@DawMatt

@DawMatt DawMatt commented Feb 23, 2024

Copy link
Copy Markdown

As per issue #75 , the existing test failed to run on Windows. The revised npm test command works on both Windows and Mac, as the removed text (node_modules/.bin) is automatically inserted at the beginning of the PATH before executing.

@fluffy-dutchman

Copy link
Copy Markdown

Couldn't you use 'npx spectral lint examples/valid/* -r ./.spectral.yml' which would also make sure that the dependent packages are installed before running the linter. Using either 'node_modules/.bin' or 'spectral' is making an assumption that the NPM package is already installed by the user. I'd suggest using 'npx spectral' in the test command instead of either of these approaches.

@fluffy-dutchman fluffy-dutchman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See previous comment about using 'npx'

@DawMatt

DawMatt commented Feb 23, 2024

Copy link
Copy Markdown
Author

I'd suggest using 'npx spectral' in the test command instead of either of these approaches.

@tnederveld , Updated as suggested via dda130d . I've retested on Windows and Mac and both work OK with this modification.

@DawMatt

DawMatt commented Mar 10, 2024

Copy link
Copy Markdown
Author

Thanks @tnederveld . I note that this PR is still stating it needs 1 approving review, from a reviewer with write access. Does this mean I need another review beyond yours? Do you have write access to this repository?

@jmlue42 , could you please clarify?

@jmlue42 jmlue42 self-assigned this Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants