User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.56 Safari/537.17 Steps to reproduce: Ran the unit test suite Actual results: The SMS application's tests failed Expected results: The SMS application's tests should have passed.
Note: the failing tests were disabled in Bug 838993, this bug is to enable them back.
Summary: [sms] Fix failing unit tests → [SMS] Fix & restore Unit Tests
Status: UNCONFIRMED → NEW
Ever confirmed: true
Now we have restored all tests (there was a missing '}' at the end :S ) removed by bug 838993 and I fixed one test which was failing. It's so important to land this asap because we need to ensure that SMS is working as expected.
It works most of the time but I add once the following error : 1) [sms] Threads-list Tests Threads-list rendering Check HTML structure: Error: expected 3 to equal 4 It appears only sometimes but we still need to fix this. we may need to add a callback to renderMessages that will be called when all messages are rendered, so that we can have an asynchronous setup in the test and call |done| when we're ready to run the tests. Also, I think we should separate MessageManager in its own file (message_manager.js maybe), so that we could mock it easily, but that could be done in Bug 841775.
I've removed the sorting method, so now the scenario is more real. I cant reproduce the error you mention before :S On the other hand we could split the code in several files for getting a cleaner structure, I will do it in bug 841775.
Comment on attachment 715049 [details] Pull Request Still get this sometimes : 1) [sms] Threads-list Tests Threads-list rendering Check HTML structure: Error: expected 2 to equal 3 at chaiAssert (http://test-agent.gaiamobile.org:8080/common/test/helper.js:30) at equal (http://test-agent.gaiamobile.org:8080/common/vendor/chai/chai.js:1250) at assertNumberOfElementsInContainerByTag (http://sms.gaiamobile.org:8080/test/unit/sms_test.js:41) at (anonymous) (http://sms.gaiamobile.org:8080/test/unit/sms_test.js:187) Really, I'm sure the problem is that we begin to test before renderMessages is finished. Could you try implementing what I said (a callback in renderMessages that will be called when all messages are rendered) ? Another way could be to have a sort of "setTimeout loop" in your setup that would check that all messages are rendered before calling "done". Sorry that I'm strict here but we can't let an error happening only sometimes as this would ruin the whole concept of the continuous integration.
(In reply to Julien Wajsberg [:julienw] from comment #6) > 1) [sms] Threads-list Tests Threads-list rendering Check HTML structure: > (...) > Really, I'm sure the problem is that we begin to test before renderMessages > is finished. It's not related with renderMessages due to the problem it's in Thread List rendering. I've updated the PR making `renderThreads` method sync. ,so the error should not be there anymore. Could you check again? Thanks
Here is the result of Travis https://travis-ci.org/mozilla-b2g/gaia/builds/4907482 , All tests passed.
I still get it : 1) [sms] Threads-list Tests Threads-list rendering Check HTML structure: Error: expected 2 to equal 3 I get it once for about 4 ou 5 runs. Just run repeatedly "APP=sms make test-agent-test" and this will happen at one moment.
Fixed. Adding you as a reviewer (finger crossed!!)
Comment on attachment 715049 [details] Pull Request r=me These tests need further rework but that will be handled in Bug 841775.
Attachment #715049 - Flags: review?(felash) → review+
Merged! https://github.com/borjasalguero/gaia/commit/0ece2d93de42578b7f54a73d924970676148ff8f https://github.com/mozilla-b2g/gaia/commit/bf98de3a646e140952849da0f0c411cbd3d7c50e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Comment on attachment 715049 [details] Pull Request NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): Tests were failing User impact if declined: More than user is that we need tests for ensure that following PR are working as expected! Testing completed: All tests working! Risk to taking this patch (and alternatives if risky): There is no risk. We need to have tests in our apps, and this patch is restoring it! ;) String or UUID changes made by this patch:
note: only tests file are touched, this would be nice to have this on both v1-train and v1.0.1 branches.
Marking as a blocker so this can be landed to v1.0.1 as well for wider test coverage.
blocking-b2g: --- → tef+
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → affected
status-b2g18: affected → fixed
status-b2g18-v1.0.1: affected → fixed
No need to create a TC in Moztrap for this defect.
Cannot verify, need steps to blackbox test this issue.
You need to log in before you can comment on or make changes to this bug.