Fix RBNF Tokenizer rule recogniser and add a NotImplementedException error subclass

arnavk commented 8 years ago

Problem

While working on #62, I realised that some RBNF rules didn't work. (Exception: Cannot call method 'rule_for' of null).

Cause

The RBNF tokenizer didn't tokenise some rules correctly. The Rule recognizer regex wasn't being generated properly because I was calling the wrong method(s) (oops!). This led to a few exceptions when querying a subset of the rules. This exception was being suppressed because the test was ignoring all exceptions in the RNBF tests rather than only the tests for rules which weren't implemented. I've added a NotImplementedException error subclass and tests only skip on these errors now.

(Should I go ahead and throw this exception everywhere in the library where stuff isn't implemented? For instance here or here)

Notes:

This branched is based off of the branch for #62 and should be merged after that one is merged. Till then the change-set won't be of much help. 😅
A downside of this is that now the tests take a really long time (there are about 30k tests). I've made this PR to address this.

@KL-7 @camertron

camertron commented 8 years ago

It's going to be a lot easier to review this after #62 is merged :smile:

arnavk commented 8 years ago

Indeed 😅

camertron commented 8 years ago

Hey @radzinzki can you update this branch now that #62 has been merged?

arnavk commented 8 years ago

@camertron, done! This should make way more sense now. 😅

Also, now that #62 is merged, I'll open my other PR against the upstream master. (I'd opened it against my master to have a small and demonstrable change-set)

Edit - Done https://github.com/twitter/twitter-cldr-js/pull/71

twitter / twitter-cldr-js