Closed brianjmiller closed 11 years ago
Will resolve #33 when done.
@davidells-scorm I'd like to go ahead and get this PR reviewed and merged to master as I believe it covers the features we need for our various TinCanJS using projects. We can come back around to the missing features when time permits.
Given the timeline and the size of this PR, I'm going to have to put trust in your testing and just hand wave it on to @bscSCORM
@brianjmiller I know you've talked about wanting more tests, but can you point out where these tests make any LRS calls at all, if that's going on I'm having trouble following. Seems like a very simple: store a statement, fetch it back test, store a state document, fetch it back test would be a big improvement.
Can you describe what your manual testing process has been?
@bscSCORM There are "okay" tests easily covering what you mention above, in the test/js/TinCan.js file. Those tests have been run against Chrome, Firefox, Safari, and IE in a number of browser versions prior to most (if not all) of this PR, but this PR should have very little to do with cross browser stuff. IOW, nothing really changed about the request model only the data and interfaces. Additionally the build script lints the code itself so we get avoidance of comma errors, etc. Having said that I've run them against Chrome for sure and can look at hitting a couple of additional browsers again. At any point you can run the tests by cloning the repo, copying the 'test/config.js.template' to 'test/config.js' and hitting 'test/index.html' if you'd like to see what is currently tested. I also plan to test the prototypes in a number of browsers, that is waiting for this to land (which is a little backwards but the best way to exercise this code).
This code is not complete and should not be merged until it is because we default to sending with the latest version when version isn't specifically configured for an LRS. But I wanted to get it out there in case anyone comes hunting, and/or anyone would like to comment on 1.0 changes ahead of release.
Requirements: