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 missing static/icons directory to debian package #5628

Merged

Conversation

DrGFreeman
Copy link
Contributor

@DrGFreeman DrGFreeman commented Nov 15, 2020

Status

Ready for review.

Description of Changes

Fixes #5625

Changes proposed in this pull request:

Add missing static/icons directory to securedrop-app-code Debian package. The directory was added in #5593 but not included in securedrop/.rsync-filter.

Testing

How should the reviewer test this PR?
Write out any special testing steps here.

  1. Run make build-debs.
  2. Check the icons are included by running the command below and verify the output equals 22.
    dpkg --contents build/xenial/securedrop-app-code_1.7.0~rc1+xenial_amd64.deb | \
        grep -e static/icons/.*\.png | wc -l
    
  3. Run make staging, visit the Journalist Interface and verify that the icons are displayed correctly.

Deployment

Any special considerations for deployment? Consider both:

  1. Upgrading existing production instances.
  2. New installs.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made changes to the system configuration:

If you made non-trivial code changes:

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

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

If you added or updated a code dependency:

Choose one of the following:

  • I have performed a diff review and pasted the contents to the packaging wiki
  • I would like someone else to do the diff review

Add missing static/icons directory to securedrop-app-code Debian package.
@DrGFreeman
Copy link
Contributor Author

Note that I am not currently setup to run the staging environment so I could not check the third item in the proposed tests.

@DrGFreeman
Copy link
Contributor Author

DrGFreeman commented Nov 16, 2020

I'm a bit puzzled as to why the focal-app-tests CI check failed in this PR as well as in #5629 while it passed in #5621 which is currently the latest change in develop.

It's curious that the fixture fails (because there are more files in the storage than expected) during the first setup and not during the following ones. Also, the fixture fails only on Focal, not on Xenial.

I wrote the fixture that is failing so I would like to understand why it fails, however, I could not run make test-focal (or make dev-focal) as it seems that there is a 404 error in Docker build when trying to reach https://www.torproject.org/dist/torbrowser/10.0.1/tor-browser-linux64-10.0.1_en-US.tar.xz ...

Update: Ran tests in Focal dev container but could not replicate the fixture setup failure.

Will keep looking for the cause but if anyone as an idea, please chime in!

@zenmonkeykstop
Copy link
Contributor

I reran the tests in #5629 and they passed - which only adds to the mystery!

Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

👍 This works. I'm going to merge it because the test flake is happening elsewhere, and will be addressed separately. Thanks again @DrGFreeman.

@rmol rmol merged commit 0b2eef5 into freedomofpress:develop Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New icons aren't included in securedrop-app-code package
4 participants