jhlywa/chess.js — pull request #21 by mattflaschen (2012-10-05 23:48:56+00:00)
Title: Tweak load_pgn tests and add failing test case showing that load_pgn does not check legality

I discovered that load_pgn does not detect (all?) invalid moves. There aren't many load_pgn test cases, and all of them were implicitly for successful cases.

So I changed it in order that each test specified whether the PGN is expected to be valid or not, then added a PGN with an invalid move. This fails as expected, since the expected result is false and the actual result is true. There is also an odd issue where the generated PGN shows a + (check) on the first move. This might be an artifact due to the invalid PGN source. I also fixed a log message.

All the previous tests still pass.

If I go back to 52fd0eebd4, the commit before the load_pgn optimization (5d31e420b1213dcc13cf9e7ca8ec9993dd2522ad, the result is false as expected. The test still fails, but only because the generated PGN is truncated before the illegal move. I'm not sure what PGN should be generated if the source PGN is illegal, so I didn't change that part of the test yet. Perhaps the pgn method should return null.

I think either:

1. load_pgn should do a full validation (including move legality) of the PGN by default (probably going back to calling generate_moves)
2. load_pgn should provide an option to do a full validation. The default is up to you.

I'm willing to help fix this, in response to your feedback.