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

Resolve deviation between dev/staging and prod #3771

Merged
merged 2 commits into from
Sep 4, 2018
Merged

Conversation

redshiftzero
Copy link
Contributor

Status

Ready for review

Description of Changes

Fixes #3758 (this is the cause of deviation between dev/staging and prod).

During #3741, we updated Flask and Werkzeug in the
securedrop-app-code package dependencies. However, these same
dependencies were not updated in the test dependencies file, which include
Flask and Werkzeug (due to the pip-compile logic we have, which
pull in Flask and Werkzeug for Flask-testing). This
meant that in dev, the older versions of these deps were used.
Even in staging, the older versions of these deps was used due
to the fact that we install the test dependencies directly
in the staging VMs after the securedrop-app-code package is
installed.

Testing

  1. Checkout 0.9.0-rc2
  2. Cherry pick in this commit
  3. make -C securedrop dev
  4. Reproduce defect 0.9.0-rc2 does not allow text only submissions #3758:

screen shot 2018-08-31 at 5 28 48 pm

This demonstrates that the source of divergence between dev and prod is resolved

Deployment

None, effects dev/staging only

That said, this should be picked into release/0.9 such that staging CI is running on the correct dependencies

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

@redshiftzero
Copy link
Contributor Author

These test failures appear to be legitimate and at least are in part (maybe totally, needs more investigation) caused by the same changes upstream that produced defect #3758 in the application code. Additional application code changes will be necessary, which means that we need to fix these issues and QA a final release candidate (rc4) on Tuesday.

@kushaldas
Copy link
Contributor

The integration tests are failing mostly because rqworker is not running, and thus the files are not getting deleted from the filesystem. I am still trying to identify why?

@kushaldas
Copy link
Contributor

Now the same integration tests are failing on the 0.8.0 too in my computer. What is going on? :(


tests/test_integration.py::TestIntegration::test_delete_collection FAILED                                                                              [ 10%]
tests/test_integration.py::TestIntegration::test_delete_collections FAILED                                                                             [ 20%]
tests/test_integration.py::TestIntegration::test_filenames PASSED                                                                                      [ 30%]
tests/test_integration.py::TestIntegration::test_filenames_delete FAILED                                                                               [ 40%]
tests/test_integration.py::TestIntegration::test_login_after_regenerate_hotp PASSED                                                                    [ 50%]
tests/test_integration.py::TestIntegration::test_reply_normal FAILED                                                                                   [ 60%]
tests/test_integration.py::TestIntegration::test_submit_file FAILED                                                                                    [ 70%]
tests/test_integration.py::TestIntegration::test_submit_message FAILED                                                                                 [ 80%]
tests/test_integration.py::TestIntegration::test_unicode_reply_with_ansi_env FAILED                                                                    [ 90%]
tests/test_integration.py::TestIntegration::test_user_change_password PASSED                                                                           [100%]
$ git log

commit e2c2f07d1ccbaebbdb54bc6b553e6c975ea8d7db
Author: redshiftzero <[email protected]>
Date:   Tue Jun 26 22:20:48 2018 +0000

    SecureDrop 0.8.0

@kushaldas
Copy link
Contributor

kushaldas commented Sep 1, 2018

I cherry picked this commit on a branch on top of develop.

I don't know what is wrong, but, for the last 6 hours all of those tests which were failing (mostly because of not being able to delete files) are now suddenly passing. I made no changes :(

@kushaldas
Copy link
Contributor

So, if I run the tests in this fix-test-pip-deps branch, those tests are failing. But, if I checkout a new branch from the develop and cherry-pick the commit 0a88f0bc8c0f204f0671e47a11d0832c79ac5ff5 from here, then all tests pass.

During #3741, we updated Flask and Werkzeug in the securedrop-
app-code package dependencies. However, these same dependencies
were not updated in the test dependencies file, which include
Flask and Werkzeug (due to the pip-compile logic we have). This
meant that in dev, the older versions of these deps were used.
Even in staging, the older versions of these deps was used due
to the fact that we install the test dependencies directly
in the staging VMs _after_ the securedrop-app-code package is
installed.
@redshiftzero
Copy link
Contributor Author

Investigated this today - the reason for the CI failures was #3773 (the fix for #3758 was not in the out of date develop this PR was based on, and at some point CI stopped rebasing on develop, leading to the confusion). I've rebased this manually on latest develop, and added d64ece5 to update safety. All tests are passing now.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

Thanks @redshiftzero for the investigation, great catch. Went through the test plan, and did reproduce the failure described.

Based on the conversation, ran the local tests on the branch and observed no failures (490 passed, 2 xfailed, 1 warnings in 3563.11 seconds)

@emkll emkll merged commit beca7a2 into develop Sep 4, 2018
@emkll emkll deleted the fix-test-pip-deps branch September 4, 2018 15:38
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.

0.9.0-rc2 does not allow text only submissions
3 participants