Closed Bug 1061513 Opened 6 years ago Closed 5 years ago

Execute jshint for tests directory

Categories

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

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ShellHacker, Assigned: ShellHacker, Mentored)

References

Details

Attachments

(1 file)

This is the main tracking bug for executing jshint tests on the various js files present in the tests directory and its subdirectories.
Depends on: 105770
Depends on: 1057770
No longer depends on: 105770
Depends on: 1061516
Depends on: 1061518
Depends on: 1061519
Depends on: 1061520
Depends on: 1061523
Depends on: 1061526
Depends on: 1061528
Depends on: 1061530
Depends on: 1061533
Depends on: 1061536
Depends on: 1061539
Depends on: 1061541
Depends on: 1061542
Depends on: 1061545
Depends on: 1061547
Depends on: 1061549
Depends on: 1061551
Depends on: 1061552
Depends on: 1061554
Depends on: 1061555
Depends on: 1061557
Depends on: 1061560
Depends on: 1061564
Depends on: 1061567
Depends on: 1061569
Depends on: 1061573
Depends on: 1061575
Blocks: 1061577
Depends on: 1061872
Depends on: 1061874
Depends on: 1061875
Depends on: 1061877
Depends on: 1061880
Blocker cleared.

.jshintignore file updated
Ready to patch dependant bugs.
Hi Sudheesh - to be honest, I think you may want to stop working on these tests until we get clarification if they are still in use. I have a feeling that we may not use them, and we can probably delete them. I will check on this now. Doing a ni? on you for awareness.
Flags: needinfo?(sudheesh1995)
Depends on: 1082329
As a reminder, the list of files that we need to fix JSHint for can be found here: https://github.com/mozilla-b2g/gaia/blob/master/build/jshint/xfail.list
Kevin,
I am not aware of if we should be deleting these files, hence I can't really say anything about that.
If we decide on deleting a few of these tests, well close the relevant bugs as FIXED.

The xfail.list has only apps/ and shared/ files, we can keep a tracker and work on those instead with a hope to clear that file.
Flags: needinfo?(sudheesh1995) → needinfo?(kgrandon)
(In reply to Sudheesh Singanamalla (:ShellHacker) from comment #4)
> Kevin,
> I am not aware of if we should be deleting these files, hence I can't really
> say anything about that.
> If we decide on deleting a few of these tests, well close the relevant bugs
> as FIXED.
> 
> The xfail.list has only apps/ and shared/ files, we can keep a tracker and
> work on those instead with a hope to clear that file.

Sounds good. It would still be useful to do the files in the tests/ directory, but just be aware of the files that might be deleted in bug 1082329. Thanks for your contributions!
Flags: needinfo?(kgrandon)
(In reply to Kevin Grandon :kgrandon from comment #5)

I enjoyed contributing so far and understood the codebase pretty well. Thanks for patiently reviewing everything so far and being great mentors. I would love to pick on the xfail.list later.

> (In reply to Sudheesh Singanamalla (:ShellHacker) from comment #4)
> > Kevin,
> > I am not aware of if we should be deleting these files, hence I can't really
> > say anything about that.
> > If we decide on deleting a few of these tests, well close the relevant bugs
> > as FIXED.
> > 
> > The xfail.list has only apps/ and shared/ files, we can keep a tracker and
> > work on those instead with a hope to clear that file.

I've cc'd myself on that bug , I'll work depending on the result of that

> Sounds good. It would still be useful to do the files in the tests/
> directory, but just be aware of the files that might be deleted in bug
> 1082329. Thanks for your contributions!
Fixes hint errors in all files except

1. tests/performance/exec-sync.js

This looks like an external file, should this also be patched, or should this be patched in the respective project's upstream.

2. tests/atoms/remote_date.js
3. tests/atoms/screenshot.js

Here is the Possible strict violation, Strict violation.
Attachment #8505387 - Flags: review?(kgrandon)
Duplicate of this bug: 1061539
Comment on attachment 8505387 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25176

Thank you for the patch. This is really close but needs a bit more work before we can land it. Please address my comments on github and re-flag me for review.
Attachment #8505387 - Flags: review?(kgrandon) → review-
Comment on attachment 8505387 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25176

Updated. Hopefully should be fine now ?
Attachment #8505387 - Flags: review- → review?(kgrandon)
Comment on attachment 8505387 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/25176

This looks good to me. Thank you for your contribution!
Attachment #8505387 - Flags: review?(kgrandon) → review+
In master: https://github.com/mozilla-b2g/gaia/commit/f72283cbe38ba0430fca62f6a25f9af20eac8c28

Sudheesh - Could you help us and remove the dependent bugs which have been landed as a part of this? Thank you for doing this!
Assignee: nobody → sudheesh1995
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(sudheesh1995)
Resolution: --- → FIXED
Thanks for reviewing it and helping me out Kevin. I've closed the dependant bugs which have landed because of this with a comment on each bug saying |Fixed by https://github.com/mozilla-b2g/gaia/commit/f72283cbe38ba0430fca62f6a25f9af20eac8c28| 

I've left the 4 Bugs which haven't been fixed with the status NEW, should I resolve them to RESOLVED WONTFIX ?

I would love to take on a new bug and clearing xfail.list seems to be a good one there. Could you guide me through that, or assign me a new bug
Flags: needinfo?(sudheesh1995) → needinfo?(kgrandon)
Blocks: 1084191
(In reply to Sudheesh Singanamalla (:ShellHacker) from comment #13)
> Thanks for reviewing it and helping me out Kevin. I've closed the dependant
> bugs which have landed because of this with a comment on each bug saying
> |Fixed by
> https://github.com/mozilla-b2g/gaia/commit/
> f72283cbe38ba0430fca62f6a25f9af20eac8c28| 
> 
> I've left the 4 Bugs which haven't been fixed with the status NEW, should I
> resolve them to RESOLVED WONTFIX ?

Sure, I've done this.


> I would love to take on a new bug and clearing xfail.list seems to be a good
> one there. Could you guide me through that, or assign me a new bug

Sounds good - there is still a bunch of work to be done there. I've filed bug 1084191 as a meta bug to track this work, and will NI? you on a few bugs - or you can file bugs for the remaining work there, but you may want to check with app authors before doing so.
Flags: needinfo?(kgrandon)
You need to log in before you can comment on or make changes to this bug.