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

First attempt to add a simple test #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andreasmarkussen
Copy link

@andreasmarkussen andreasmarkussen commented Dec 25, 2021

This is an attempt to add a very simple unit test to the nrlint rules section of NR Lint.

I have not added it to Dev,

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

This is my first attempt at creating a simple unit test for NR Lint rules.

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 25, 2021

CLA Signed

The committers are authorized under a signed CLA.

@knolleary
Copy link
Member

Thanks for the PR @andreasmarkussen.

As we don't currently have any unit tests in this repo we first need to get the basic structure in place for how we want to arrange the tests.

I'm keen that we have some consistency with the other repos in the node-red org. On that basis, I'd suggest:

  • All tests should be in a top-level test folder - with a folder structure that mirrors the src structure.

    That means the test you've added should be in test/lib/rules/no-duplicate-html-in-url.spec.js

  • We use should rather than chai elsewhere. I'm not sure why/how chai came to be listed as an existing dev dependency, it certainly isn't used anywhere I can see in the repo. It may be a legacy dependency from the very early development of the linter.

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.

2 participants