Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TBB to functional tests #4347

Merged
merged 8 commits into from
May 11, 2019
Merged

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Apr 15, 2019

Status

Ready for review

Description of Changes

This is an update of the work done on the tbb-0.9.0 and tbb-0.12.0 branches to run our Selenium tests using the Tor Browser.

Fixes #4290.

Testing

Run the test suite as usual. All tests should pass.

To confirm that Tor Browser is in use, connect to the Docker container's VNC port with make -C securedrop docker-vnc while the functional tests are running.

Deployment

Most of the changes are restricted to test environments.

Checklist

If you made changes to the server application code:

  • Linting (make ci-lint) and tests (make -C securedrop test) pass in the development container

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

@rmol rmol force-pushed the tbb-4290 branch 2 times, most recently from 84f6597 to 1845b55 Compare April 17, 2019 03:18
@kushaldas
Copy link
Contributor

First run sees two random failures, trying again.

platform linux2 -- Python 2.7.12, pytest-3.3.2, py-1.5.2, pluggy-0.6.0 -- /usr/bin/python
cachedir: tests/.cache
rootdir: /home/kdas/code/securedrop/securedrop/tests, inifile: pytest.ini
plugins: mock-1.7.1, cov-2.5.1
collected 21 items                                                                                                                                           

tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_interface PASSED                                                              [  4%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_edits_hotp_secret FAILED                                                      [  9%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_deletes_user PASSED                                                           [ 14%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_updates_image PASSED                                                          [ 19%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_ossec_alert_button PASSED                                                           [ 23%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_adds_admin_user PASSED                                                        [ 28%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_verifies_deletion_of_one_submission_modal PASSED                                  [ 33%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_uses_col_delete_collection_button_modal PASSED                                    [ 38%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_uses_index_delete_collections_button_modal PASSED                                 [ 42%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_interface_ui_with_modal FAILED                                                    [ 47%]
tests/functional/test_make_account_changes.py::TestMakeAccountChanges::test_admin_edit_account_html_template_rendering PASSED                          [ 52%]
tests/functional/test_source.py::TestSourceInterface::test_lookup_codename_hint PASSED                                                                 [ 57%]
tests/functional/test_source_notfound.py::TestSourceInterfaceNotFound::test_not_found PASSED                                                           [ 61%]
tests/functional/test_source_session_timeout.py::TestSourceSessions::test_source_session_timeout PASSED                                                [ 66%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_appears_if_tor_browser_not_in_use PASSED                     [ 71%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_appears_if_orbot_is_used PASSED                              [ 76%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_high_security PASSED                                         [ 80%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_submit_and_retrieve_happy_path PASSED                               [ 85%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_source_cancels_at_login_page PASSED                                 [ 90%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_source_cancels_at_submit_page PASSED                                [ 95%]
tests/functional/test_submit_and_retrieve_message.py::TestSubmitAndRetrieveMessage::test_submit_and_retrieve_happy_path PASSED                         [100%]

@kushaldas
Copy link
Contributor

platform linux2 -- Python 2.7.12, pytest-3.3.2, py-1.5.2, pluggy-0.6.0 -- /usr/bin/python
cachedir: tests/.cache
rootdir: /home/kdas/code/securedrop/securedrop/tests, inifile: pytest.ini
plugins: mock-1.7.1, cov-2.5.1
collected 21 items                                                                                                                                           

tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_interface PASSED                                                              [  4%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_edits_hotp_secret PASSED                                                      [  9%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_deletes_user PASSED                                                           [ 14%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_updates_image PASSED                                                          [ 19%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_ossec_alert_button PASSED                                                           [ 23%]
tests/functional/test_admin_interface.py::TestAdminInterface::test_admin_adds_admin_user PASSED                                                        [ 28%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_verifies_deletion_of_one_submission_modal PASSED                                  [ 33%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_uses_col_delete_collection_button_modal PASSED                                    [ 38%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_uses_index_delete_collections_button_modal PASSED                                 [ 42%]
tests/functional/test_journalist.py::TestJournalist::test_journalist_interface_ui_with_modal PASSED                                                    [ 47%]
tests/functional/test_make_account_changes.py::TestMakeAccountChanges::test_admin_edit_account_html_template_rendering PASSED                          [ 52%]
tests/functional/test_source.py::TestSourceInterface::test_lookup_codename_hint PASSED                                                                 [ 57%]
tests/functional/test_source_notfound.py::TestSourceInterfaceNotFound::test_not_found PASSED                                                           [ 61%]
tests/functional/test_source_session_timeout.py::TestSourceSessions::test_source_session_timeout PASSED                                                [ 66%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_appears_if_tor_browser_not_in_use PASSED                     [ 71%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_appears_if_orbot_is_used PASSED                              [ 76%]
tests/functional/test_source_warnings.py::TestSourceInterfaceBannerWarnings::test_warning_high_security PASSED                                         [ 80%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_submit_and_retrieve_happy_path PASSED                               [ 85%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_source_cancels_at_login_page PASSED                                 [ 90%]
tests/functional/test_submit_and_retrieve_file.py::TestSubmitAndRetrieveFile::test_source_cancels_at_submit_page PASSED                                [ 95%]
tests/functional/test_submit_and_retrieve_message.py::TestSubmitAndRetrieveMessage::test_submit_and_retrieve_happy_path PASSED                         [100%]

Second run, all green locally.

@kushaldas
Copy link
Contributor

I ran the Python3 based tests 5 times, twice test_admin_edits_hotp_secret failed. This one seems to be following the history of the TBB branch :(

Otherwise things look great. I will go through the text diff now.

@zenmonkeykstop
Copy link
Contributor

@kushaldas how would you feel about marking test_admin_edits_hotp_secret as xfail, and waiting until #3844 is in and the flaky plugin can be added before un xfailing it?

@kushaldas
Copy link
Contributor

@kushaldas how would you feel about marking test_admin_edits_hotp_secret as xfail, and waiting until #3844 is in and the flaky plugin can be added before un xfailing it?

That can one way, right now @rmol is fixing a few other old artifacts.

@rmol rmol force-pushed the tbb-4290 branch 2 times, most recently from 0e49951 to 7ba94b5 Compare April 22, 2019 14:04
@rmol rmol requested a review from zenmonkeykstop as a code owner April 29, 2019 16:14
@codecov-io
Copy link

codecov-io commented Apr 29, 2019

Codecov Report

Merging #4347 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4347   +/-   ##
========================================
  Coverage    83.76%   83.76%           
========================================
  Files           44       44           
  Lines         2950     2950           
  Branches       321      321           
========================================
  Hits          2471     2471           
  Misses         402      402           
  Partials        77       77

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 ac5c921...3919551. Read the comment docs.

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs rebase + specially for dockerfiles and docs. Worked for me this time very well.

@redshiftzero
Copy link
Contributor

@kushaldas: before we have @rmol do another rebase which is tricky given the size of this PR, want to do a review of the diff to see if there are any major issues that should be cleaned up prior to merge into develop?

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's rebase, snip out the additional/unintentional changes noted in my inline comments (mostly the diff in changelog-related commits and an isort config file), and then merge this in. We'll sort out the external server testing (i.e. testing the TB8 based functional tests work against an external staging server) in a followup once this is in develop

@@ -31,11 +31,6 @@ def test_index(self):
self._source_visits_source_homepage()
self._screenshot('source-index.png')

def test_index_javascript(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed due to flakiness?

@rmol rmol force-pushed the tbb-4290 branch 2 times, most recently from 278ca19 to 3919551 Compare May 10, 2019 21:32
stupid utility to create an test admin user fast

Working tbselineum tests for most part.

fixes two typos in readme of tor based tests

Fixes directory path in README

Hardcoded onion addresse work

Test for tor broser asking high security in slider

Reads instance information from a json file

Uses clean variable name

Configure the sleep amount between clicks

Just tests admin/journalist login and cookies

Tests working once again.

Use firefox to login for downloads in selenium tests

Updates README

Matches with 0.6-rc2 functional tests

We are using TBB 7.5

We need more time and click to test logout

Fixes the assert statement

Updates based on current upstream

TB functional tests: Gitignore instance config

This prevents the accidental commit of private information.

TB functional tests: Merge tbselenium dir

Updates the ansible files for tbb

Updates to the Dockerfile for tbb

Updates as suggested in the PR review for ansible and dockerfile

Missing tor key

Removes duplicated 'when' line from app-test logic

The 'when' conditional detecting a grsec kernel, used for running the
paxctl commands on the TBB binary, was needlessly duplicated on the
relevant task. Fortunately that didn't cause breakage, because the
'when' lines were identical, but only one was active.

Updates functional test container image

Now installs Firefox 52 ESR, rather than Firefox 46, for use inside the
test container.

Includes changes to run-test shell script:

    TOR_FORCE_NET_CONFIG=0 is required to directly connect to Tor
    network, otherwise it will wait for userinput to either connect
    or to configure

    The `run_xvfb` invocation is no longer necessary, since the test suite
    code bootstraps the headless server now.

Creates local test server inside the container

Bootstrapping the application services within the functional test suite.
Includes some cleanup, culling unused debugging code, and also cleans up
the various print statements.

Ignore functional test firefox logs (thanks, @msheiny!)

Adds retries for tor network connection failure, using the pre-existing
logic.

Uses nc rather than torsocks in functional tests

The version of torsocks in the Trusty repos isn't recent enough to
support custom ports. Rather than install from other sources, which
requires manual package verification (or configuring non-trusty repos,
which could break other packages), let's fall back to good ol' nc.

Creates proper orbot specific project to test

We need to create a new firefox profile to test the orbot specific
warning. This works for both locally and over Tor.

Updates test_make_account_changes for Tor

Now we can safely execute the account changes in the tests running
on the Tor browser. The logic update makes sure to create different
user for this test than any other test.

Reduces sleep durations in functional tests

We don't have to sleep for too long if we are running against
local instance. The ultimate goal remains to remove hardcoded sleeps
altogether, but we'll circle back and eliminate those calls once the
test suite is passing reliably.

Increases CircleCI timeout to 20m

The functional tests can take a long time, so let's instruct CircleCI to
continue waiting, to give the test suite a chance to finish
successfully.

Updates README for the functional tests

Mostly correcting a typo in the `instance_information.json` config
filename, but also updated some of the example commands. The notes
regarding potentially failing tests also seemed out of date, as several
members of the team have confirmed working functional tests under the
new TB Selenium logic recently.

Removes temporary testing related directories

We need to clean up any temporary test directory before running
any test, as the previous can create a bad state in the database.
For example, one of our pages-layout test adds 123456 as hotp value
to the test account, and it will never be able to login again.

Adds missing steps for pages-layout tests over tor

We can now generate thousands of random journalist names using
the generator. Only the first 3 names are used in the functional
tests, the rest are being used in the pages-layout.

We also added back _source_delete_key function for one the test.
Rest of the updates are to add sleep function calls or to have
better error message.

Adds comment about the user generator and lint fixes

We have get_journalist_usernames generator which can generate
unlimited number of users for many tests.

Moves around all driver creation functions into one place

We now have the functional/functional_test.py to handle all
driver creation logic. For the pages-layout tests, we are creating
only a Firefox driver to connect to the local container itself.

This logic does not work against any external server as of now.

Updates test user creation logic inside of container

Removing old method calls from user creation logic, this is only
used inside of the container for the functional testing.

Downloads data from server using requests over Tor

The test requirements now have requests[socks] as dependency.
Using the same we are now directly downloading the files/messages
from the .onion address for functional tests.

The old external command file also got removed this committ.

We are creating the gpg object for both container based local
testing and external testing (in functional tests).
Fixes: freedomofpress#3691 freedomofpress#3687

Removed xvfb, tor browser, and firefox installs from app-staging - functional tests now run remotely

fix to pass make ci-lint

Cleans up flake8 errors in functional tests

Resolves some lingering flake8 formatting violations that were causing
lint checks to fail. These changes are unrelated to the current PR, but
better late than never.

Written by @msheiny, committed by @conorsch during branch collab.

Signed-off-by: Conor Schaefer <[email protected]>

Adds --staging flag create-dev-data.py for tests

We now have --staging flag to the create-dev-data.py script so
that we can easily create an user in the staging or prodcution test
and then use the functional tests to test the instance.

Add fact gatherer for extracting tor app onion details

This is really for functional testing in staging environments, but the
script doesnt hurt to be installed in prod. It doesn't elevate
permissions or expose any sensitive details - you need to run as root in
order to gain useful data.

Logic to dump app tor onion data to func config

This commit adds logic to the test runner so that a functional test json
config gets dumped for usage in the tbb selenium test tools against
staging.

Wire-up current app-test role to our upgrade scenario

Without this change, the upgrade scenario would utilize the app-test
logic from the old repo. Which is not what we want. This is of course
kind of "hacky". I welcome the opportunity to improve this with feedback
:)

Move tor fact logic from tor role --> app-test role

I'm not sure we are ready to shove this on prod instances AND I was
running into weird old/new role logic under the molecule scenario.
Ideally the fact should be in both roles but I dunno.. this seemed like
an easier short-term fix.

ansible spacing' and tag nits

Testinfra: Update test dependencies for app-staging

In freedomofpress#3697, we removed the application testing pip dependencies.
This commit updates the testinfra test variables accordingly.

Fix linting failures

One of these was introduced in freedomofpress#3672, but not discovered due to
other CI failures (e.g. python not found when running the lint job)

Use absolute pathing in i18n testing

I'm hoping this shakes out some really weird test failures we were
seeing only under CircleCI only under the functional testing branch at a
certain point in time. Really wild behavior. *fingers crossed*

Set selenium webdriver test output to WARNING

Originally was on DEBUG and was sending out mountains of output into the
pytest process which made it difficult to assess anything.

(cherry picked from commit a1f0134)

removed duplicate entry in test requirements

Dockerfile: Update Tor Browser to 8.0

Docker development environment: Update Tor signing key

Dockerfile: Update geckodriver and firefox-esr

Swap x11vnc with tightvncserver

Basically installed this because it can be used with pyvirtualdisplay as
a backend AND because it brings in the Xvnc tooling which will start an
X11 server as well as a VNC server.

Wire-up VNC server and helper command for func tests

Had to remove x11 display logic inside test scaffolding (initially tried
to integrate it there but it kept building and destroying the VNC server
per test).

Made a VNC helper command with support for GNOME desktop and macOS (havent
tested it on mac yet). Updated the docs

Bump functional test sleep time

10 seconds is way too short.. 160 seconds.. maybe too long? Fingers
crossed I can work with the team to get the wait_for logic running

Add functionality to prepare boxes for functional testing

Typically these actions were done manually but lets get our good old
friend ansible to run them for us (at least under the upgrade env).

Added auth to VNC in test container, for OS X compatibility.

Bump TBB/ESR to 8.0.1 and 60.2.0esr combo

https://blog.torproject.org/new-release-tor-browser-801

Updated geckodriver to 0.22.0

added ini file to get around remote-viewer password prompt

Functional tests: xfail test_warning_appears_if_tor_browser_not_in_use

Due to defect freedomofpress#3793, when using Firefox Quantum, the incorrect message
is displayed on the source interface. This test will not pass until that
is resolved.

Functional tests: Fix firefox path

Testinfra: Fix test failure due to non-DRY variables files

staging.yml is a concatenation of multiple other variables files,
one was updated during rebase, one was not.

Testinfra: Update Flask version to 1.0.2

fixed VNC port being defined twice when running 'make dev'

Tests: Modify viewport size for parity between dev and CI env

In CI we are getting MoveTargetOutOfBoundsException, but not locally.
We have had errors in the past due to different viewport sizes in CI
and locally, so setting this to a standard size for the pages layout
tests (where the exception is occurring).

Functional tests: Resolve NoAlertPresentException

We were getting a NoAlertPresentException due to new behavior in
geckodriver [0] where interacting with the driver closes the modal.
Thus, we do not need to explicitly accept the modal here.

[0] mozilla/geckodriver#1171

Replaced parameterized time.sleep()s with self.waitfor()s

Stability fixes layout tests, added new functests from develop, updated TBB

fixed flaky page layout tests that were broken by long fr_FR strings causing UI elements to wrap.

fixed flake8, added explicit scroll to elements before click, stability fixes

Dockerfile: get key from Mozilla keyserver

For whatever reason, this Firefox signing key was not available on
the keyserver in the prior diff, but was available on Mozilla's
keyserver.

deps: Update requests due to CVE-2018-18074

functional tests: Add wait_for prior to clicking submit

functional tests: Remove sleeps and reduce flakiness around modal

functional tests: Remove sleeps around js alerts

functional tests: use sleep_time as default timeout

functional tests: Remove remainder of time.sleeps in source steps

Don't clobber existing custom logo

Minimal changes to remove external server testing functionality
Update Selenium and tbselenium. In the container update the Tor Browser
Bundle to 8.0.8, Firefox to 60.6.1esr (the same version used in TBB),
and geckodriver to 0.24.

Increase the shared memory available to the dev/test container.

Use tbselenium's USE_RUNNING_TOR configuration; start Tor in run-test.

There were a lot of flaky tests. Selenium seems to have particular
difficulty with clicking when buttons or forms have click or submit
event handlers.

Be more explicit about switching between Tor Browser and Firefox in tests.

Move dev/test VNC port to 5909 to coexist with staging. Rename make
target for this to docker-vnc.

Update pip-tools requirement.

Add a log format for pytest.
rmol added 6 commits May 11, 2019 12:17
It prevents the attempt in app-test/defaults/main.yml to locate the SD
root using "git rev-parse --show-toplevel".
Use a CSS selector instead of iterating links.
Since we have separate testing of all translations, and Arabic covers
both translation and RTL, we can speed up the regular suite by
dropping French. (Désolé, c'est purement pragmatique!)
Restore reverted test-config target in securedrop/Makefile; had lost
the change from iteritems->items.

Restore molecule/ from develop.

Take a sledgehammer to journalist login steps: try logging in up to
ten times to get Selenium to see the JS after page load.

Fix import fumbles from rebase
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving for merge, thanks for your patience on this @rmol. Let's carefully monitor CI on develop as given the significant changes here we might need to rapidly fix flakes if we see them in the application test jobs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port TBB functional testing support from tbb-0.9.0 to develop
5 participants