password-strength-meter.js should have unit tests

[jorbin]

As discussed in ​IRC, our goal is to have unit testable javascript. The​password-strength-meter.js file will serve as our initial testbed for testing.

This file was chosen since there will likely be some changes to it in 3.7 and it is small and already largely testable.

[jorbin]

My patch is up and it contains four tests. Based on nacin's comments during IRC that we would likely be switching to zxcvbn.js for the internals of this function, I didn't test as much as could because what is a "good" password now may not be one with the improved checking.

I suggest using this in combination with the patch up on #25096 to make the testing easier.

[adamsilverstein]

this works great and is a nice template for writing future tests. thanks! i'm almost afraid to ask, why are you leaving off semicolons for each line?

[jorbin]

Replying to adamsilverstein:

this works great and is a nice template for writing future tests. thanks! i'm almost afraid to ask, why are you leaving off semicolons for each line?

Semicolons are redundant. ​http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding

[aaroncampbell]

Replying to jorbin:

Semicolons are redundant. ​http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding

One of the things mentioned in that article was that some linters, while working fine with no semi-colons, have a hard time with the comma first approach. It looks like you use that format, have you had any issues with linting?

[jorbin]

Replying to aaroncampbell:

One of the things mentioned in that article was that some linters, while working fine with no semi-colons, have a hard time with the comma first approach. It looks like you use that format, have you had any issues with linting?

Using jshint with laxcomma set to true it is never a problem.

[aaroncampbell]

Replying to jorbin:

Using jshint with laxcomma set to true it is never a problem.

Sweet! I didn't know about the laxcomma option. For anyone interested: ​http://www.jshint.com/docs/options/#laxbreak

[nacin]

Replying to jorbin:

Semicolons are redundant. ​http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding

Sure, but please follow existing core style. This also includes commas at the end of statements.

[jorbin]

With redundant semicolons and early commas

[jorbin]

patch updated to match the styles requested by Mr. Nacin.

[jorbin]

I've updated the tests based on the addition of zxcvbn in [25157]

[jorbin]

Based on duck_'s suggestion, I added a bunch (34) of test passwords from zxcvbn ​https://github.com/lowe/zxcvbn/blob/master/test/test.coffee

[iandunn]

It'd also be good to have a test to check the user_input field to make sure that any extra words that are passed are penalized. ​Currently that's broken because of the modifications that were made to zxcvbn, so a unit test would help make sure that doesn't happen again.

I'm uploading a patch with that added. Note that the test currently fails, which is the expected behavior since Jon's latest patch hasn't been committed yet.

password-strength-meter.js also needs some reworking around the JS coding standards -- it's using spaces instead of tabs, missing whitespace around parenthesis, etc -- but I don't have time for that tonight.

[nacin]

In 25165:

Move PHPUnit tests into a tests/phpunit directory.

wp-tests-config.php can/should reside in the root of a develop checkout. phpunit should be run from the root.

see #25088.

[nacin]

In 25166:

Add QUnit to WordPress core for JavaScript unit testing.

props jorbin.
see #25088.

[nacin]

In 25167:

Add QUnit tests for password-strength-meter.js.

props jorbin.
see #25088.

[iandunn]

Was there something wrong with 5.diff?

[nacin]

Replying to iandunn:

Was there something wrong with 5.diff?

I didn't notice it. Can you refresh?

[iandunn]

Adds 'username in password should be penalized' test

[iandunn]

Replying to nacin:

Replying to iandunn:

Was there something wrong with 5.diff?

I didn't notice it. Can you refresh?

Done.

[duck_]

In 25175:

Test that passwords containing the username are penalised.

Tidy up some spelling, indentation and whitespace whilst we're at it.

Props iandunn. See #25088.

[jorbin]

Replying to duck_:

In 25175:

Test that passwords containing the username are penalised.

Tidy up some spelling, indentation and whitespace whilst we're at it.

Props iandunn. See #25088.

Thanks for the spelling corrections :)

I think we can close this since we do now have unit tests and that any new tests for password-strength-meter.js we wish to add can be new tickets.

[duck_]

Replying to jorbin:

I think we can close this since we do now have unit tests and that any new tests for password-strength-meter.js we wish to add can be new tickets.

Indeed.

[TobiasBg]

Just one more clean-up/spacing/typo patch in 25088-spacing.patch.

[iandunn]

Replying to TobiasBg:

Just one more clean-up/spacing/typo patch in 25088-spacing.patch.

Nice, I think it's much more readable that way :)

In the "username in password should be penalized" test, though, I think variables that aren't assigned a value are supposed to be on a single line: ​http://make.wordpress.org/core/handbook/coding-standards/javascript/#the-var-keyword

[TobiasBg]

Good call, Ian! Thanks! I refreshed the patch.

[nacin]

It seems that jQuery uses });, not } );, and that seems to be very common for jQuery-like code (including our JS), so we should probably continue to do that.

Our (and jQuery) style also appears to be i++, not ++i. There is a space added after a + at the end of the diff, but not before it. (Look for '+.) Looks good otherwise!

[TobiasBg]

also changes } ); to });

[TobiasBg]

Argh, those +es :-)
Refreshed patch fixes ++i and the '+.

I'm still unsure about the }); to be honest though... That kind of contradicts spaces in brackets.

test( 'whatever', function() { ... });

looks wrong.
However, 25088-spacing-braces.patch will change that, too, whereas 25088-spacing.patch will not.

[SergeyBiryukov]

In 25275:

Whitespace cleanup for password strength meter tests. props TobiasBg. fixes #25088.

[SergeyBiryukov]

In 25294:

Use correct paths in multisite.xml. see [25165]. see #25088.