Closed brianjmiller closed 10 years ago
@mnutz can you have a look to make sure this makes sense. Primarily subjective difference, but I like this approach slightly better as it doesn't rely on checking the main JObject for an 'objectType' which I agree should always exist, I'm just not convinced that it actually will. (It will from an LRS, but I could see using the API from elsewhere that it wouldn't.)
I added a unit test case that fails originally, and then passes with your change as well as my switch.
I think this is the best solution, because SubStatement is not derived from Statement, so it seems to discard nested substatements. @brianjmiller, can you add a test case with a nested one?
@mnutz I knew you were going to ask that ;-). I'll add one.
Enough testing improvement for the moment. I'm content that going forward additional tests will be easier to construct given the new Support class (and no more copy+pasta). I'd still like to get the LRS configuration pulled out into a separate file but my Googling didn't present me with an immediate, easy fix there so I'm at least including the Travis CI creds we use elsewhere.
@mnutz please have a look, but I'm going to set this in front of @bscSCORM for consideration and merge. Once merged we'll need to drop in a new assembly version and do a proper release, but I'll be out until next week so won't be able to do that until then (assuming this gets merged). Presumably you are good to go for testing/work based on this.
@bscSCORM what say you?
Adjusts work from @mnutz slightly and starts Statement testing.