-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SPIKE: Test framework for recommendations feature #154
Comments
Jordan and I did a brief spike on this and will attach a PR to this card. We spent 30 minutes on the spike. We disabled all existing tests, and were able to add a new test module for Riff that builds under the existing target. We verified that we can create new unit test cases (did not check integration testing), and that test cases can both pass and fail in expected ways. New tests can easily be added as files under the Note: The existing test suite, although now disabled, actually is only failing on ~35 of 650 tests. Ideally, we would take a couple of days to address this, and then we'd have full test coverage of the code base, rather than only partial coverage of the portion we create new tests for. Further, by writing separate test cases, we are generating additional technical debt that will increase the long we go without fixing the original test suite. We will make a new card for fixing the 35 broken tests in the original suite and merging them with any riff tests. Overall, we are good to go on unit tests for the new feature set. |
We have unit testing working now (pending merging of this PR). Unit testing allows us to easily test whether any single technical part of a feature is working correctly. So, for example, we can test that, if you give a certain date to the part of the code that generates recommendations, it does produce the expected list. What we cannot do right now is integration tests, where we mockup the entire UI programmatically, and actually test that if we simulate setting the time on the simulated computer to a certain date, the information displayed on a portion of the simulated screen matches the expected value. We could set up mocks for integration testing (we have checked that the test suite supports this), but this would a considerable undertaking. We estimate at least 2 more days, with a lot of potential for more. We think unit testing is a solid standard, if we can build with functioning unit tests, that would actually be a big improvement on the reliability over existing code, and think it is not worth it to try to get integration tests. Let us know if you are okay with this. We can take tomorrow to work on trying to get integration testing, but are not optimistic. |
Assigned code review to @mlippert |
Thank you @jaedoucette and @jordanreedie - this is super helpful! I agree, unit tests seem reasonable and like a good step in the right direction. |
No code to merge from this spike. Marking as accepted last sprint. @juliariffgit - please change that status to another version of accepted / closed if need be. |
As a PM, I want to release a recommendations feature to test the concept of Riff feedback based on our analytics. Because this is a prominent user-oriented feature that is mean to showcase Riff as an "intelligent" tool that directs people into behaviors that will make them more successful, it is especially important that it not be (or look) broken to the user, which would undermine their confidence in the quality of Riff as a product.
Because we don't currently have a functioning test framework with which we can execute unit tests, or integration tests, I would like to investigate the possibility of getting something in place very quickly, so have at least minimal test coverage for this feature before we deploy it for the NEXT course in September.
** Spike Acceptance Criteria **
A meeting with the team by 8/13 (or sooner) to report on the following:
The text was updated successfully, but these errors were encountered: