Update pyvirtualdisplay #831

MollieBakal · 2020-12-14T02:10:35Z

Closes #694

… and default_manager_params.json (openwpm#822) * Deleted redundant files and references to - default_browser_params.json and default_manager_params.json * added validate_crawl_configs to docs * updated the docs on how to instantiate TaskManager * Updated Using_OpenWPM.md * Updated Platform-Architecture.md

…enWPM into post-rename-fixes

forgot to pull across fork

codecov · 2020-12-14T02:28:47Z

Codecov Report

Merging #831 (fc7c772) into CommandRefactoring (4d7db91) will not change coverage.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##           CommandRefactoring     #831   +/-   ##
===================================================
  Coverage               42.22%   42.22%           
===================================================
  Files                      29       29           
  Lines                    3214     3214           
===================================================
  Hits                     1357     1357           
  Misses                   1857     1857

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d7db91...1ac71d3. Read the comment docs.

MollieBakal · 2020-12-14T02:45:18Z

Linking the issue #694 -- also, please note the first 5 commits here were already merged

Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.8. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.8) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

vringar

Thank you for creating this PR! I've left a couple of minor things and will comment some more on the issue.

environment.yaml

openwpm/deploy_browsers/deploy_firefox.py

test/test_mp_logger.py

vringar · 2020-12-14T13:35:14Z

test/test_xvfb_browser.py

+from functools import partial
+from typing import List
+
+from openwpm.command_sequence import CommandSequence
+from openwpm.task_manager import TaskManager
+
+from .openwpmtest import OpenWPMTest
+from .utilities import BASE_TEST_URL
+
+
+class TestXVFBDisplay(OpenWPMTest):
+    """Test the XVFB display option to see if it runs and shuts down properly after a command is given"""
+
+    def get_config(self, data_dir=""):
+        return self.get_test_config(data_dir, display_mode="xvfb")
+
+    def test_display_shutdown(self):
+        manager_params, browser_params = self.get_config()
+        TEST_SITE = BASE_TEST_URL + "/test_pages/simple_a.html"
+        manager = TaskManager(manager_params, browser_params)
+
+        sequence = CommandSequence(TEST_SITE)
+        sequence.get()
+        sequence.save_screenshot()
+        manager.execute_command_sequence(sequence)
+        manager.close()


I don't think that this test tests anything that isn't tested by test_simple_commands.py

As per request later, added different command. Had not seen that displaymode=xvfb was tested, sorry.

vringar · 2020-12-14T13:47:16Z

When writing this comment I noticed one thing.
If we could guarantee that the Display object always gets stoped then we could just remove all of the other code.
And since the entire function is already wrapped in a try/except-block we could add a finally that contains the changes you made in the shutdown block.
This way, no matter how we exit the function we'd stop the Display properly.

Would you want to do this?
Could you also rebase this PR on the CommandRefactoring branch and then create a test with a custom command that throws an exception, to see if the lock file gets removed properly, even when there is an exception.

… and default_manager_params.json (openwpm#822) * Deleted redundant files and references to - default_browser_params.json and default_manager_params.json * added validate_crawl_configs to docs * updated the docs on how to instantiate TaskManager * Updated Using_OpenWPM.md * Updated Platform-Architecture.md

…enWPM into post-rename-fixes

MollieBakal · 2020-12-15T04:38:40Z

These commits aren't done--I'm having some trouble with the environment

MollieBakal · 2020-12-15T05:22:02Z

By rebasing based on CommandRefactoring, it seems to have created a number of unintended commits--I don't know if that was unintentional, or if I should file another pull request to that branch instead of master.

…enWPM into post-rename-fixes

openwpm/deploy_browsers/deploy_firefox.py

test/test_mp_logger.py

test/test_xvfb_browser.py

vringar · 2020-12-15T11:31:04Z

By rebasing based on CommandRefactoring, it seems to have created a number of unintended commits--I don't know if that was unintentional, or if I should file another pull request to that branch instead of master.

I have changed this PR to target the CommandRefactoring branch. Unfortunately this is always a bit messy with GitHub, but I'll ignore all files that I know you didn't touch.

vringar · 2020-12-15T11:31:40Z

These commits aren't done--I'm having some trouble with the environment

If you elaborate on what these troubled are, I'd be happy to help you.

Co-authored-by: Stefan Zabka <zabkaste@informatik.hu-berlin.de>

MollieBakal · 2020-12-15T14:08:48Z

Thanks, but I think it was just an issue with cached files? I reran install.sh and that seems to have cleared it up.

test/test_xvfb_browser.py

vringar · 2020-12-16T16:59:52Z

Thanks for fixing this issue :)
I'll wait for @englehardt to merge the CommandRefactoring into master before I merge this.
Do you want remove the __init__ in the meantime?

Co-authored-by: Stefan Zabka <zabkaste@informatik.hu-berlin.de>

MollieBakal · 2020-12-16T18:55:34Z

Yep! Thank you

MollieBakal and others added 11 commits December 2, 2020 20:24

manual test breaking post-openwpm#807

25fe8e9

replacing with standard json function for single string

d112642

modifying test to see if stops running for over two hours:

79aa598

Reverting error, updating function to current name

b08dfe1

Merge branch 'post-rename-fixes' of https://github.com/MollieBakal/Op…

6bab765

…enWPM into post-rename-fixes

Attempted fix treats display as object

e694157

changing display; pytest unchanged

abcd7d1

added a test case to ensure xvfb runs

5795552

simplifying test

65d010b

Merge pull request #2 from mozilla/master

6afb484

forgot to pull across fork

dependabot bot added 3 commits December 14, 2020 12:03

vringar self-requested a review December 14, 2020 11:17

vringar requested changes Dec 14, 2020

View reviewed changes

vringar changed the title ~~update of pyvirtualdisplay as per #694~~ Updated of pyvirtualdisplay Dec 14, 2020

MollieBakal and others added 10 commits December 14, 2020 23:23

manual test breaking post-openwpm#807

1255cdb

replacing with standard json function for single string

9832dae

modifying test to see if stops running for over two hours:

a90269f

Attempted fix treats display as object

62daf22

changing display; pytest unchanged

4757896

added a test case to ensure xvfb runs

774e4d6

simplifying test

641f168

fixing requested issues

bfede3f

Merge branch 'post-rename-fixes' of https://github.com/MollieBakal/Op…

bcb16fb

…enWPM into post-rename-fixes

runs; modified test to include exception

b693700

MollieBakal added 3 commits December 15, 2020 01:35

Merge branch 'master' into post-rename-fixes

ebe318e

fix import

f0bba54

Merge branch 'post-rename-fixes' of https://github.com/MollieBakal/Op…

e622378

…enWPM into post-rename-fixes

MollieBakal changed the title ~~Updated of pyvirtualdisplay~~ Update pyvirtualdisplay Dec 15, 2020

MollieBakal requested a review from vringar December 15, 2020 07:39

vringar changed the base branch from master to CommandRefactoring December 15, 2020 10:32

vringar requested changes Dec 15, 2020

View reviewed changes

openwpm/deploy_browsers/deploy_firefox.py Outdated Show resolved Hide resolved

test/test_mp_logger.py Show resolved Hide resolved

test/test_xvfb_browser.py Show resolved Hide resolved

Queue not optional; removed

f20e6c3

Co-authored-by: Stefan Zabka <zabkaste@informatik.hu-berlin.de>

updated test to check for lockfile

e15acf8

vringar reviewed Dec 16, 2020

View reviewed changes

test/test_xvfb_browser.py Outdated Show resolved Hide resolved

vringar self-requested a review December 16, 2020 16:58

MollieBakal and others added 2 commits December 16, 2020 13:52

Update test/test_xvfb_browser.py

43aa5d1

Co-authored-by: Stefan Zabka <zabkaste@informatik.hu-berlin.de>

sticky key in the last commit, same change

1ac71d3

vringar approved these changes Dec 18, 2020

View reviewed changes

vringar closed this Jan 9, 2021

vringar deleted the branch openwpm:CommandRefactoring January 9, 2021 10:15

vringar mentioned this pull request Jan 11, 2021

Pyvirtualdisplay update #844

Merged

vringar mentioned this pull request Feb 22, 2021

Update pyvirtualdisplay to >1.0.0 version #694

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update pyvirtualdisplay #831

Update pyvirtualdisplay #831

MollieBakal commented Dec 14, 2020 •

edited by vringar

Loading

codecov bot commented Dec 14, 2020 •

edited

Loading

MollieBakal commented Dec 14, 2020

vringar left a comment

vringar Dec 14, 2020

MollieBakal Dec 15, 2020

vringar commented Dec 14, 2020 •

edited

Loading

MollieBakal commented Dec 15, 2020

MollieBakal commented Dec 15, 2020

vringar commented Dec 15, 2020

vringar commented Dec 15, 2020

MollieBakal commented Dec 15, 2020

vringar commented Dec 16, 2020

MollieBakal commented Dec 16, 2020

Update pyvirtualdisplay #831

Update pyvirtualdisplay #831

Conversation

MollieBakal commented Dec 14, 2020 • edited by vringar Loading

codecov bot commented Dec 14, 2020 • edited Loading

Codecov Report

MollieBakal commented Dec 14, 2020

vringar left a comment

Choose a reason for hiding this comment

vringar Dec 14, 2020

Choose a reason for hiding this comment

MollieBakal Dec 15, 2020

Choose a reason for hiding this comment

vringar commented Dec 14, 2020 • edited Loading

MollieBakal commented Dec 15, 2020

MollieBakal commented Dec 15, 2020

vringar commented Dec 15, 2020

vringar commented Dec 15, 2020

MollieBakal commented Dec 15, 2020

vringar commented Dec 16, 2020

MollieBakal commented Dec 16, 2020

MollieBakal commented Dec 14, 2020 •

edited by vringar

Loading

codecov bot commented Dec 14, 2020 •

edited

Loading

vringar commented Dec 14, 2020 •

edited

Loading