Closed Bug 934952 Opened 11 years ago Closed 11 years ago

Add test-agent coverage report into allow-failures of Travis CI

Categories

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::TestAgent
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: rickychien, Assigned: rickychien)

Details

Attachments

(2 files)

Pull request link for gaia
199 bytes, text/html
julienw
: review+
Details
Pull request link for js-test-agent
52 bytes, text/x-github-pull-request
julienw
: review+
Details | Review
User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release) Build ID: 20130910160258
Summary: Added test-agent coverage report into allow-failures of Travis CI → Add test-agent coverage report into allow-failures of Travis CI
Assignee: nobody → ricky060709
Attachment #827784 - Flags: review?(yurenju.mozilla)
Comment on attachment 827784 [details] Pull request link for gaia ><html> > <head> > <meta http-equiv="Refresh" > content="1; url=https://github.com/mozilla-b2g/gaia/pull/13408" /> > </head> > <body> > Redirect to pull request link... > </body> ></html>
Attachment #827784 - Attachment mime type: text/plain → text/html
Comment on attachment 827784 [details] Pull request link for gaia we only have browser coverage report because it failed on browser unit test, could you ignore failed and get coverage report for all gaia apps? thanks.
Attachment #827784 - Flags: review?(yurenju.mozilla)
I agree it is a better way to ignore fail and continue to run testing, but the reason caused testing stop is because mocha. Runing mocha and encounter some of fails are going to stop testing. For example: https://travis-ci.org/RickyChien/gaia/jobs/13561239 (here is without blanket.js) So, I will trace the mocha to understand what kind of fails caused it to stop.
I found this is a bug when integrating blanket.js and it have been fixed now.
Attachment #827784 - Attachment description: Pull request link → Pull request link for gaia
Attachment #833421 - Flags: review?(yurenju.mozilla)
Attachment #827784 - Flags: review?(yurenju.mozilla)
The step to use this patch is: 1. Apply gaia patch 2. Remove test-agent "rm -rf /tools/test-agent/node_moudles/test-agent" 3. "git clone git@github.com:RickyChien/js-test-agent test-agent" and move test-agent to /tools/test-agent/node_moudles/ 4. "git checkout -b issue-934952 origin/issue-934952" 5. run "test-agent-test COVERAGE=1"
Comment on attachment 833421 [details] [review] Pull request link for js-test-agent Ricky, please describe your change in git commit message, looks this change doesn't relate to add coverage report into allow-failures of travis.
Attachment #833421 - Flags: review?(yurenju.mozilla)
Attachment #833421 - Flags: review?(yurenju.mozilla)
I'm pretty busy those days. Julien, are you available to review this pr?
Flags: needinfo?(felash)
Attachment #827784 - Flags: review?(yurenju.mozilla)
Attachment #833421 - Flags: review?(yurenju.mozilla)
Attachment #827784 - Flags: review?(felash)
Attachment #833421 - Flags: review?(felash)
Yep, taking the reviews, will help me understand better the test-agent internals ;) If I don't feel comfortable I'll ask you or James back.
Flags: needinfo?(felash)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Here is the result of gaia PR on travis-CI without new js-test-agent: https://travis-ci.org/RickyChien/gaia/jobs/14238775 We can see all modules coverage report as well as some test log on console. Getting neat coverage output if enable the new version js-test-agent.
I've added some comments on the pull request. Sorry for the delay, I was busier than expected, but now that I know your patch better I'll be able to react faster.
I've leaved comments on PR too. You can check it ASAP. :)
Ricky, I just want to say I'm sorry it takes so much time, I was very busy with blocker bugs... I promise I'll check this again during this week-end.
I added a comment and question on the github PR.
Component: General → Gaia::TestAgent
@Julien Thank you for reviewing. I also leave comment in github PR :)
Hi Julien, Do you need help to review? :-)
Flags: needinfo?(felash)
Hey Yuren, if your queue is shorter now, then you can take the review back, as obviously I haven't be able to move it forward yet. I still don't like to have this boolean "useCoverage" in all functions. What if we want a third feature in the future? Ricky, can we use an "options" object like "{ coverage: true }" instead of a boolean argument ? With this change to your code I would agree to move forward.
Flags: needinfo?(felash)
Hi Julien, I've updated my PR and you can check it again. Console output from allowed failures in travis is at https://travis-ci.org/mozilla-b2g/gaia/jobs/17899518 that currently shows no result because I found there are blanket loading script error in browser app.
Flags: needinfo?(felash)
(In reply to Ricky Chien [:rickychien] from comment #19) > Console output from allowed failures in travis is at > https://travis-ci.org/mozilla-b2g/gaia/jobs/17899518 that currently shows no > result because I found there are blanket loading script error in browser app. Do you have another bug for this? I have the same issue locally and thus I can't test :(
Flags: needinfo?(felash)
(In reply to Julien Wajsberg [:julienw] from comment #20) > (In reply to Ricky Chien [:rickychien] from comment #19) > > > Console output from allowed failures in travis is at > > https://travis-ci.org/mozilla-b2g/gaia/jobs/17899518 that currently shows no > > result because I found there are blanket loading script error in browser app. > > Do you have another bug for this? I have the same issue locally and thus I > can't test :( I'm going to find out what's problem and fix in this bug.
Ok thanks, tell me when you find it !
Current travis-ci report at https://travis-ci.org/mozilla-b2g/gaia/jobs/18711822 was stuck in search. Due to https://github.com/mozilla-b2g/gaia/blob/master/apps/search/test/unit/providers/marketplace_test.js#L7 require app_provider.js but it isn't exist! So, blanket throw error to stop instrumenting. I will find out all similar errors and fixed in this patch soon.
I'm fine in this patch if this throws, but in my test it was not running at all :) If it's actually running (even failing) with your new patch then I'm fine. Let's file other bugs for these issues.
Hi, there too many errors in our tests. xd I determine to modify blanket to ignore all of these script loading errors.
Yeah, probably the best thing to do... but in another bug ? ;)
Now we can see more coverage https://travis-ci.org/mozilla-b2g/gaia/jobs/18778744 But another problem arised by communication that block blanket.
I think it's time to land this feature due to almost all modules were covered.
Agreed, let's land this
Comment on attachment 833421 [details] [review] Pull request link for js-test-agent r=me
Attachment #833421 - Flags: review?(felash) → review+
Comment on attachment 827784 [details] Pull request link for gaia r=me
Attachment #827784 - Flags: review?(felash) → review+
Comment on attachment 833421 [details] [review] Pull request link for js-test-agent For this one, we'll need to bump the version for test-agent so that the changes in lib/node/bin/test.js are used. Therefore I don't know what these changes are doing?
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: