password-manager-resources icon indicating copy to clipboard operation
password-manager-resources copied to clipboard

Simple test framework using Mocha, initial tests for validating JSON files

Open billjive opened this issue 4 years ago • 11 comments

Checklist

  • [x] The PR isn't documenting something that would be a common practice among password managers (e.g. minimal length of 6)
  • [x] The top-level JSON objects are sorted alphabetically
  • [x] There are no open pull requests for the same update.

for websites-with-shared-credential-backends.json

  • [x] There's evidence the domains are related (SSL certificates, DNS entries, valid links between sites, legal documents etc.)
  • [x] The new group serves login pages on each of the included domains, and those login page accept accounts from the others. (For example, we don't associate google.co.il to google.com, because google.co.il redirects to accounts.google.com for authentication.)

for change-password-URLs.json

  • [ ] There is no Well-Known URL for Changing Passwords (https://example.com/.well-known/change-password)

I thought it would be useful to add a basic testing framework to this project. It uses Mocha and is run locally with node/npm. I added a package.json file which has basic info but also dev dependencies (just Mocha and another library to validate URLs). To run, do "nom install" followed by "nom test" in the main directory.

Current tests just validate the basics in the JSON files. All entries are validate to be valid URLs, for example.

Future work could be to validate the password rules per the WHATWG spec.

billjive avatar Jun 08 '20 23:06 billjive

Oh, neat. @igor-makarov, what do you think?

rmondello avatar Jun 09 '20 04:06 rmondello

Looking good! I have a few notes:

  1. I'd have the tests for different quirk files in separate source files, for easier diffing/merging.
  2. Some of the Ruby-based validations in #130 & #151 can be rewritten in JS so that the test suite stays homogenous in terms of language/framework.
  3. Validating password rules can be done using the tools/PasswordRulesParser.js parser instead of regex, am I right @rmondello?
  4. Let's add it to GitHub Actions!

igor-makarov avatar Jun 09 '20 06:06 igor-makarov

@billjive if you rebase, I can add the GitHub-based workflow to your branch!

igor-makarov avatar Jun 09 '20 08:06 igor-makarov

@billjive if you rebase, I can add the GitHub-based workflow to your branch!

Great! Thanks everyone for the feedback, good suggestions. I can get this going in a day or two, sound good?

billjive avatar Jun 09 '20 22:06 billjive

@billjive if you rebase, I can add the GitHub-based workflow to your branch!

Ok -- all rebased. I also split up the quirks tests in to 3 (well, now 4) files. I added a sanity check test case on the new 2fa json file.

I looked at the other PR's and I can easily add a test case to check for duplicate entries, etc. Any other ideas?

billjive avatar Jun 10 '20 04:06 billjive

@billjive I made a PR to your branch with the addition of the test action. See it run here: https://github.com/igor-makarov/password-manager-resources/actions

igor-makarov avatar Jun 10 '20 05:06 igor-makarov

Great! Merged..

billjive avatar Jun 10 '20 06:06 billjive

@billjive This is really cool. It looks like it might need to be rebased. I was looking over the dependencies JSON file. That’s a ton of dependencies, no?

rmondello avatar Aug 05 '20 20:08 rmondello

@rmondello thanks -- it's been a while so yes I'm sure it needs to be rebased. I'll figure out too if there are too many dependencies, not sure at the moment but will look. I'll get to this stuff over the weekend I think.

I also rewrote your ruby test suite in javascript. It was a bigger rewrite and I can share with you in a different branch. Works just the same!

billjive avatar Aug 06 '20 14:08 billjive

@billjive Very interested in seeing the Ruby->JavaScript re-write!

rmondello avatar Aug 06 '20 15:08 rmondello

@rmondello I posted details here: https://github.com/apple/password-manager-resources/issues/98#issuecomment-671488154

billjive avatar Aug 11 '20 15:08 billjive