Closed Bug 1274167 Opened 8 years ago Closed 8 years ago

Add Linter (flake8) support for Firefox-ui and Puppeteer

Categories

(Testing :: Firefox UI Tests, defect)

Product:
Testing
Component:
Firefox UI Tests
Version:
49 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox51 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
Tracking Status
firefox51 --- fixed

People

(Reporter: whimboo, Assigned: sinha.piyush0609, Mentored)

References

(
URL
)

Details

(Whiteboard: [lang=py][good first bug])

Attachments

(1 file, 4 obsolete files)

Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.
58 bytes, text/x-review-board-request
whimboo
: review+
Details 
With bug 1270506 we got linter support for mach which includes flake8. It would be good to see this added for the following source tree folders:

/testing/firefox-ui/
/testing/puppeteer/

To be able to get our files run through the linter via `mach lint [%folder%]` the above mentioned paths have to be added to the linter configuration which is located in /tools/lint/flake8.lint.

Before a patch can be accepted we have to get all listed failures by the above command fixed.
Assign me please!
That's great. I will do so immediately. I'm looking forward to it, and if you have questions please let me know.
Assignee: nobody → jacob.harrowmortelliti
Status: NEW → ASSIGNED
Is this the desired output for 'mach lint /testing/firefox-ui'?

> flake8: No files to lint for specified paths!
> ✖ 0 problems (0 errors, 0 warnings)
Flags: needinfo?(hskupin)
Please check the referenced bug in comment 0. It explains or shows how to add folders to the lint checks.
Flags: needinfo?(hskupin)
(In reply to Henrik Skupin (:whimboo) from comment #4)
> Please check the referenced bug in comment 0. It explains or shows how to
> add folders to the lint checks.

I did that, and I added the paths, but I was checking to make sure it's right.  Seems like if we are going to add the paths, they should have stuff to check.
Everything submitted okay?
Flags: needinfo?(hskupin)
Jacob, the addition looks fine but were you able to get passing results for all of our directories? From my initial check a lot of failures should popup.
Flags: needinfo?(hskupin)
(In reply to Henrik Skupin (:whimboo) [away 06/06 - 06/10] from comment #8)
> Jacob, the addition looks fine but were you able to get passing results for
> all of our directories? From my initial check a lot of failures should popup.

mach lint /testing/puppeteer
mach lint /testing/firefox-ui
mach lint /tools/lint
mach lint /python/mozlint

all return:
>flake8: No files to lint for specified paths!
>✖ 0 problems (0 errors, 0 warnings)
Flags: needinfo?(hskupin)
(In reply to Jacob Mortelliti[:earlgreyhot] from comment #9)
> mach lint /testing/puppeteer
> mach lint /testing/firefox-ui
> mach lint /tools/lint
> mach lint /python/mozlint

The paths you are specifying here are not correct. `/testing` is actually referencing a `testing` folder under the root of your root partition. What you want here is `testing/firefox-ui`. I checked that myself and actually got 20 problems listed. Let me know if you still have problems with it.
Flags: needinfo?(hskupin)
Jacob, please let me know if you still have issues to get it working correctly.
Flags: needinfo?(jacob.harrowmortelliti)
It looks like that Jacob is no longer around, which means that we need another assignee for this bug.
Assignee: jacob.harrowmortelliti → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(jacob.harrowmortelliti)
Comment on attachment 8756654 [details]
MozReview Request: Bug 1274167 -  Add Linter (flake8) support for Firefox-ui and Puppeteer. r=whimboo.

https://reviewboard.mozilla.org/r/55322/#review58886

Resetting review request given that this patch has not been finished yet.
Attachment #8756654 - Flags: review?(hskupin)
(In reply to Henrik Skupin (:whimboo) from comment #10)
> (In reply to Jacob Mortelliti[:earlgreyhot] from comment #9)
> > mach lint /testing/puppeteer
> > mach lint /testing/firefox-ui
> > mach lint /tools/lint
> > mach lint /python/mozlint
> 
> The paths you are specifying here are not correct. `/testing` is actually
> referencing a `testing` folder under the root of your root partition. What
> you want here is `testing/firefox-ui`. I checked that myself and actually
> got 20 problems listed. Let me know if you still have problems with it.


Henrik,

I tried the following path: ./mach lint testing/firefox-ui

I'm getting the following:

wpt: No files to lint for specified paths!
flake8: No files to lint for specified paths!
✖ 0 problems (0 errors, 0 warnings)
Have you made the necessary changes to `tools/lint/flake8.lint` first? If not flake8 will not automatically check our folders.
I made the necessary changes to 'tools/lint/flake8.lint' by adding testing/firefox-ui' and 'testing/puppeteer' to the 'include' directory. I tried the following path: ./mach lint testing/firefox-ui 

I'm getting the following:
Could not find flake8! Install flake8 and try again.

    $ pip install flake8
wpt: no files to lint in specified paths
✖ 0 problems (0 errors, 0 warnings)

Any reason as to why I'm getting this message?
As far as I remember there is a bug when specifying the exact path. But I'm not fully sure. Andrew might be able to help you. Also be aware that I will not be around the next 7 days.
(In reply to Ryan Nath [:rnath] from comment #16)
> I made the necessary changes to 'tools/lint/flake8.lint' by adding
> testing/firefox-ui' and 'testing/puppeteer' to the 'include' directory. I
> tried the following path: ./mach lint testing/firefox-ui 
> 
> I'm getting the following:
> Could not find flake8! Install flake8 and try again.
> 
>     $ pip install flake8
> wpt: no files to lint in specified paths
> ✖ 0 problems (0 errors, 0 warnings)
> 
> Any reason as to why I'm getting this message?

I believe this means it's working :). The wpt message just means that the wpt linter didn't find any files to lint under that directory. But the wpt linter is unrelated to what you care about. I put those messages in so people didn't falsely assume that their directory is error free when instead it just wasn't being linted at all.. but an argument can be made that they are more confusing/annoying than anything else. If you wish to run only the flake8 linter, you can use:
./mach lint --linter flake8 testing/firefox-ui testing/puppeteer

Try introducing a lint violation in that folder on purpose to verify that flake8 finds and reports it.
No, we should have tons of warnings to fix. It's not that we are that perfect. :) I think running the command without the path should show the warnings?
Oh, hmm.. it should work with the path too. Fwiw I see the errors when running:
./mach lint testing/firefox-ui

locally (after adding that path to the .lint file)
Oh, I see.. Sorry Ryan, I thought that was two separate commands there. You need to have flake8 installed.
Ryan, are you still interested to work on this bug?
Flags: needinfo?(nathmatics)
I am interested to solve this bug.
Piyush, given that Ryan didn't respond yet feel free to get started to work on this bug. Once a patch has been uploaded for review I will mark the bug as assigned to you. Thank you actually for your interest!
Flags: needinfo?(nathmatics)
Cas a good first bug, I would like to know:
1) where to get the code?
2) do we submit a pull request to github, or something in bugzilla as an attachment?
3) which commands would we run after getting the code to see the error?
(In reply to Piyush Sinha from comment #25)
> Cas a good first bug, I would like to know:

Sure, and sorry that those haven't been provided yet. Let me know if the information below satisfies it all.

> 1) where to get the code?

You can get the code when cloning: http://hg.mozilla.org/mozilla-central/

Then check comment 0 for which files need to be updated to add coverage of the tests.

Not sure if you want to work with Mercurial or git. For the latter you could use git-cinnabar to also clone hg repositories but would take some extra time to setup: https://github.com/glandium/git-cinnabar

> 2) do we submit a pull request to github, or something in bugzilla as an
> attachment?

You will have to submit the patch as best via mozreview. How to setup and submit you can read here:
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install.html

> 3) which commands would we run after getting the code to see the error?

See the docs in how to run our tests:
https://developer.mozilla.org/en-US/docs/Mozilla/QA/Firefox_UI_tests
Thanks Henrik, this is very helpful.
You'll also need to run |mach lint|, see https://gecko.readthedocs.io/en/latest/tools/lint/index.html for more information.
wpt: no files to lint in specified paths
testing/firefox-ui/harness/firefox_ui_harness/__init__.py
  7:1  error  module level import not at top of file  E402 (flake8)
  7:1  error  'cli_functional' imported but unused    F401 (flake8)
  8:1  error  module level import not at top of file  E402 (flake8)
  8:1  error  'cli_update' imported but unused        F401 (flake8)

testing/firefox-ui/harness/firefox_ui_harness/arguments/__init__.py
  5:1  error  'firefox_ui_harness.arguments.base.FirefoxUIArguments' imported but unused  F401 (flake8)
  6:1  error  'firefox_ui_harness.arguments.update.UpdateArguments' imported but unused   F401 (flake8)

testing/firefox-ui/harness/firefox_ui_harness/runners/__init__.py
  5:1  error  'firefox_ui_harness.runners.base.FirefoxUITestRunner' imported but unused  F401 (flake8)
  6:1  error  'firefox_ui_harness.runners.update.UpdateTestRunner' imported but unused   F401 (flake8)

testing/firefox-ui/tests/functional/security/test_mixed_content_page.py
  5:1  error  'marionette_driver.Wait' imported but unused  F401 (flake8)

testing/firefox-ui/tests/functional/sessionstore/test_restore_windows_after_restart.py
  129:5  error  too many blank lines (2)  E303 (flake8)

testing/firefox-ui/tests/puppeteer/test_appinfo.py
  6:1  error  'marionette_driver.errors.MarionetteException' imported but unused  F401 (flake8)

testing/firefox-ui/tests/puppeteer/test_menubar.py
  16:9  error  local variable 'num_tabs' is assigned to but never used  F841 (flake8)

testing/firefox-ui/tests/puppeteer/test_page_info_window.py
  5:1  error  'marionette_driver.By' imported but unused  F401 (flake8)

testing/firefox-ui/tests/puppeteer/test_windows.py
  6:1  error  'marionette_driver.errors.TimeoutException' imported but unused  F401 (flake8)

✖ 14 problems (14 errors, 0 warnings)
[envy@envy mozilla-central]$ ./mach lint testing/puppeteer
wpt: no files to lint in specified paths
testing/puppeteer/firefox/firefox_puppeteer/__init__.py
  5:1  error  'os' imported but unused  F401 (flake8)

testing/puppeteer/firefox/firefox_puppeteer/testcases/__init__.py
  5:1  error  'firefox_puppeteer.testcases.base.BaseFirefoxTestCase' imported but unused  F401 (flake8)

testing/puppeteer/firefox/firefox_puppeteer/testcases/base.py
  42:13  error  block comment should start with '# '  E265 (flake8)
  46:80  error  whitespace before ','                 E203 (flake8)
  51:60  error  whitespace before ','                 E203 (flake8)
  51:93  error  whitespace before ','                 E203 (flake8)

testing/puppeteer/firefox/firefox_puppeteer/ui/browser/window.py
   8:1  error  'marionette_driver.errors.NoSuchWindowException' imported but unused      F401 (flake8)
  11:1  error  'marionette_driver.keys.Keys' imported but unused                         F401 (flake8)
  13:1  error  'firefox_puppeteer.api.l10n.L10n' imported but unused                     F401 (flake8)
  14:1  error  'firefox_puppeteer.api.prefs.Preferences' imported but unused             F401 (flake8)
  15:1  error  'firefox_puppeteer.decorators.use_class_as_property' imported but unused  F401 (flake8)
  28:1  error  redefinition of unused 'errors' from line 5                               F811 (flake8)
  28:1  error  'firefox_puppeteer.errors' imported but unused                            F401 (flake8)

testing/puppeteer/firefox/firefox_puppeteer/ui/update_wizard/__init__.py
  5:1  error  'dialog.UpdateWizardDialog' imported but unused  F401 (flake8)

✖ 14 problems (14 errors, 0 warnings)

Running |mach lint| for both the folders produces 14 errors which I have to fix. Right?
This output looks exactly what I expected. So yes, those errors you will have to get fixed. Thanks!
Now please assign the bug to me :)
Why flake8 shows error import but unused?
Any guesses because when i commented out the imports and ran the test, these imports are used and flake8 shows error.Any idea?
Dear Henrik, I have fixed the issue and tests are also passed.Kindly review the code changes.
__init__.py files in every module contains some imports which are considered unused by flake8. Ignoring the statements which are shown as unused imports.
(In reply to Piyush Sinha from comment #33)
> Created attachment 8777574 [details]
> "BUG 1274167" Added Linter(flake8) support for Firefox-ui and Puppeteer
> 
> Review commit: https://reviewboard.mozilla.org/r/69118/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/69118/

This commit didn't work so don't your time in reviewing it.
Ignored the shown as unused imports by flake8 using # noqa.

Review commit: https://reviewboard.mozilla.org/r/69254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69254/
Hi Piyush, if we have files which flake8 should not lint, we should add those to the exclusion list. This can be done by adding a custom .flake8 file to our root folder. Here an example as what Marionette is using: https://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/.flake8
Two more hints for mozreview:

1. Please squash the individual commits into a single one as described in the documentation. 

2. When you want to ask for review please add `r?whimboo` at the end of the commit message. Mozreview will then automatically set the review flag.

Thanks
Assignee: nobody → sinha.piyush0609
Status: NEW → ASSIGNED
Attachment #8756654 - Attachment is obsolete: true
https://reviewboard.mozilla.org/r/69116/#review66334

::: tools/lint/flake8.lint:131
(Diff revision 1)
>          'tools/lint',
>          'taskcluster',
>          'testing/marionette/client',
>          'testing/talos/',
> +        'testing/firefox-ui',
> +        'testing/puppeteer',

Please try to keep the entries sorted alphabetically so it will be easier to spot later.
https://reviewboard.mozilla.org/r/69254/#review66336

::: gecko.log:1
(Diff revision 1)
> +1470307173716	Marionette	INFO	Listening on port 2828

You added the gecko.log by accident. Lets get it removed.

::: testing/firefox-ui/harness/firefox_ui_harness/__init__.py:8
(Diff revision 1)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  __version__ = '1.4.0'
>  
> -# import cli_functional
> -# import cli_update
> +import cli_functional # noqa
> +import cli_update  # noqa

Better add the custom .flake8 file and exclude it in general.

::: testing/firefox-ui/tests/functional/security/test_mixed_content_page.py:5
(Diff revision 1)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> -# from marionette_driver import Wait
> +from marionette_driver import Wait # noqa

If Wait is not used here remove this line. This also applies to other changes.

::: testing/firefox-ui/tests/puppeteer/test_menubar.py:16
(Diff revision 1)
>  
>      def setUp(self):
>          FirefoxTestCase.setUp(self)
>  
>      def test_click_item_in_menubar(self):
> -        # num_tabs = len(self.browser.tabbar.tabs)
> +        num_tabs = len(self.browser.tabbar.tabs) # noqa

It's not used. Remove it.
Henrik, I added the .flake8 file to the firefox-ui directory 
and running mach lint on it produces this result
wpt: no files to lint in specified paths
flake8: no files to lint in specified paths

I only excluded __init__.py file.

Any suggestions?
You get that message if none of the paths you specified are configured to be linted with |mach lint|. So don't forget to add "testing/firefox-ui" and "testing/puppeteer" to tools/lint/flake8.lint. Then run something like:

./mach lint --linter flake8 testing
linter support is working for firefox-ui and puppeteer

r?whimboo

Review commit: https://reviewboard.mozilla.org/r/69288/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/69288/
Piyush, can you please squash all the commits into a single one? The current situation is not really reviewable. Also the `r?` part has to be put at the end of the first line, and not at the very end. Further small nit: `Bug` and not `BUG` in the commit message. Thanks.
I don't know how to squash commits into single one.
Help me out
Would you mind to join us on IRC (irc.mozilla.org) which would help a lot for conversations like those. If you don't have a local client, you could use https://kiwiirc.com/client/irc.mozilla.org/#automation. You will find me in the automation channel under the nickname whimboo.
Dear Henrik,
https://reviewboard.mozilla.org/r/69114/diff/3#index_header you can see squashed diff from this page
So this is still not a squashed diff given that mozreview still shows 4 individual changesets (d1a28ab51ce7 to fed1a7e7b229). You have to get rid of all of them, so only a single one remains. As best with the mozreview-id of the first changeset (d1a28ab51ce7).

I would suggest that we completely solve this on IRC first before you are uploading more patches.
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69116/diff/1-2/
Attachment #8777573 - Attachment description: included folders for linter(flake 8) support → Bug 1274167-Add linter(flake8) support for firefox-ui and puppeteer;
Attachment #8777573 - Flags: review?(hskupin)
Attachment #8777574 - Attachment is obsolete: true
Attachment #8777782 - Attachment is obsolete: true
Attachment #8777836 - Attachment is obsolete: true
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review66688

::: testing/firefox-ui/.flake8:2
(Diff revision 2)
> +[flake8]
> +max-line-length = 99

I thought that this line length is set by default. Maybe I was wrong. Andrew, mind replying?

::: testing/firefox-ui/harness/firefox_ui_harness/__init__.py:8
(Diff revision 2)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  __version__ = '1.4.0'
>  
> -import cli_functional
> -import cli_update
> +import cli_functional 
> +import cli_update  

Nearly all the lines you touched before and which have been reverted, have trailing spaces now. Please check to get those changes reverted too. Usually a local lint run should have complained about that. Did you do one?

This applies to also all other files.

::: testing/firefox-ui/tests/puppeteer/test_appinfo.py:6
(Diff revision 2)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  import mozversion
> -from marionette_driver.errors import MarionetteException
> +from marionette_driver.errors import MarionetteException  # noqa

I think you missed to remove this line?
Attachment #8777573 - Flags: review?(hskupin) → review-
https://reviewboard.mozilla.org/r/69116/#review66688

> I thought that this line length is set by default. Maybe I was wrong. Andrew, mind replying?

No, linting the files shows error so this line is to be included.

> Nearly all the lines you touched before and which have been reverted, have trailing spaces now. Please check to get those changes reverted too. Usually a local lint run should have complained about that. Did you do one?
> 
> This applies to also all other files.

Local lint does not show any error.

> I think you missed to remove this line?

I 'll do that asap.
(In reply to Henrik Skupin (:whimboo) from comment #52)
> ::: testing/firefox-ui/.flake8:2
> (Diff revision 2)
> > +[flake8]
> > +max-line-length = 99
> 
> I thought that this line length is set by default. Maybe I was wrong.
> Andrew, mind replying?

It is set by default in the global .flake8 file, however flake8 doesn't have any inheritance mechanism between config files. So if you want to set other non-standard config options, you'll need to re-add max-line-length. If you don't need any other non-standard config options, simply delete testing/firefox-ui/.flake8 and it will pick up the global max-line-length automatically.
I see. Thanks for the reply. In that case I think we would also have to add the following line from the global .flake8 file:

https://dxr.mozilla.org/mozilla-central/source/.flake8:
> filename = *.py, +.lint
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69116/diff/2-3/
Attachment #8777573 - Flags: review- → review?(hskupin)
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review67216

There is one thing still to do as pointed out below. Otherwise I will trigger a Try build so we can see if the linter job and our tests are passing.

::: testing/firefox-ui/harness/firefox_ui_harness/__init__.py:8
(Diff revision 3)
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  __version__ = '1.4.0'
>  
> -import cli_functional
> -import cli_update
> +import cli_functional 
> +import cli_update  

Please really fix those trailing spaces in all the touched files as pointed out in my last review.
Attachment #8777573 - Flags: review?(hskupin) → review-
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review67664

The patch looks fine to me now. Thank you for all the updates! Anyway, I triggered another try build to verify that all is fine.
Attachment #8777573 - Flags: review?(hskupin) → review+
Piyush, would you mind rebasing your changes for latest HEAD on mozilla-central? You will expect a merge conflict. Once this is and the commit message is fixed I can go ahead and get this pushed out. Thanks.
How (In reply to Henrik Skupin (:whimboo) from comment #62)
> Piyush, would you mind rebasing your changes for latest HEAD on
> mozilla-central? You will expect a merge conflict. Once this is and the
> commit message is fixed I can go ahead and get this pushed out. Thanks.

How should I do it?
(In reply to Piyush Sinha from comment #63)
> How (In reply to Henrik Skupin (:whimboo) from comment #62)
> > Piyush, would you mind rebasing your changes for latest HEAD on
> > mozilla-central? You will expect a merge conflict. Once this is and the
> > commit message is fixed I can go ahead and get this pushed out. Thanks.
> 
> How should I do it?

i dont have scm_level_3 to land commit.
I didn't request that you should push the patch, instead it needs an update regarding the latest changes on mozilla-central. So pull from that branch and fix the merge issues. Once the patch got updated on this bug (incl the commit message) I will land it.
(In reply to Henrik Skupin (:whimboo) from comment #65)
> I didn't request that you should push the patch, instead it needs an update
> regarding the latest changes on mozilla-central. So pull from that branch
> and fix the merge issues. Once the patch got updated on this bug (incl the
> commit message) I will land it.

I think I checked it and I am seeing no merge issues.
(In reply to Piyush Sinha from comment #66)
> (In reply to Henrik Skupin (:whimboo) from comment #65)
> > I didn't request that you should push the patch, instead it needs an update
> > regarding the latest changes on mozilla-central. So pull from that branch
> > and fix the merge issues. Once the patch got updated on this bug (incl the
> > commit message) I will land it.
> 
> I think I checked it and I am seeing no merge issues.

Sorry about that. but can you please tell me which changes are you referring to because I can see errors while running test.I need to know about the changes which were made specifically to /testing/firefox-ui and testing/puppeteer.
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review69188

I've made an inline comment to what specifically needs to be removed. Please check it.

::: testing/firefox-ui/tests/puppeteer/test_windows.py:6
(Diff revision 6)
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  from marionette_driver import By, Wait
> -from marionette_driver.errors import NoSuchWindowException, TimeoutException
> +from marionette_driver.errors import NoSuchWindowException

The TimeoutException has already been removed by the patch on bug 1290186. So this should cause a merge conflict. Please remove that file from the diff.
(In reply to Henrik Skupin (:whimboo) from comment #68)
> Comment on attachment 8777573 [details]
> Bug 1274167-Add linter(flake8) support for firefox-ui and puppeteer;
> 
> https://reviewboard.mozilla.org/r/69116/#review69188
> 
> I've made an inline comment to what specifically needs to be removed. Please
> check it.
> 
> ::: testing/firefox-ui/tests/puppeteer/test_windows.py:6
> (Diff revision 6)
> >  # This Source Code Form is subject to the terms of the Mozilla Public
> >  # License, v. 2.0. If a copy of the MPL was not distributed with this
> >  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> >  
> >  from marionette_driver import By, Wait
> > -from marionette_driver.errors import NoSuchWindowException, TimeoutException
> > +from marionette_driver.errors import NoSuchWindowException
> 
> The TimeoutException has already been removed by the patch on bug 1290186.
> So this should cause a merge conflict. Please remove that file from the diff.

I updated my repository and applied the patch which was reviewed. Now I am not seeing any changes made to the test_windows.py.Do you want me to push it?
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review69188

> The TimeoutException has already been removed by the patch on bug 1290186. So this should cause a merge conflict. Please remove that file from the diff.

Please check if this is the change you were talking about.
Comment on attachment 8777573 [details]
Bug 1274167 - Add Linter(flake8) support for Firefox-ui and Puppeteer.

https://reviewboard.mozilla.org/r/69116/#review69188

> Please check if this is the change you were talking about.

This looks fine now. Thanks.
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/791641aca0f8
Add Linter(flake8) support for Firefox-ui and Puppeteer.r=whimboo
https://hg.mozilla.org/mozilla-central/rev/791641aca0f8
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Piyush, thank you a lot for working on this bug and getting your very first patch landed! It will help us a lot in the future to keep our tests in shape. If you enjoyed the work, and if you want to learn more, then please get in contact with me (maybe better via IRC) and we can find a new bug for you.
(In reply to Henrik Skupin (:whimboo) from comment #76)
> Piyush, thank you a lot for working on this bug and getting your very first
> patch landed! It will help us a lot in the future to keep our tests in
> shape. If you enjoyed the work, and if you want to learn more, then please
> get in contact with me (maybe better via IRC) and we can find a new bug for
> you.

Sure, I will be in touch with you. I learned a lot while working on this bug.
(In reply to Piyush Sinha from comment #77)
> (In reply to Henrik Skupin (:whimboo) from comment #76)
> > Piyush, thank you a lot for working on this bug and getting your very first
> > patch landed! It will help us a lot in the future to keep our tests in
> > shape. If you enjoyed the work, and if you want to learn more, then please
> > get in contact with me (maybe better via IRC) and we can find a new bug for
> > you.
> 
> Sure, I will be in touch with you. I learned a lot while working on this bug.

Hey, I am not able to connect to to the irc channel automation through kiwi.Is there any problem with kiwi?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: