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

Test tor packages from tor-apt-test in release branches #2941

Merged
merged 5 commits into from Jan 30, 2018

Conversation

msheiny
Copy link
Contributor

@msheiny msheiny commented Jan 26, 2018

Status

Work in progress

Description of Changes

Partially fixes #2919 -- performs testing against CI staging AFTER installing latest tor package in tor-apt-test.freedom.press

Changes proposed in this pull request:

Ummm what I described right above this part... GEEEZ!!

Testing

How should the reviewer test this PR?

You want to check to make sure CI passed and confirm you are able to see a .tor_version artifact in circle CI and it's content matches version 0.3.2.9-1~trusty+1

Deployment

Any special considerations for deployment? Consider both:

Well this PR affects CI specfically, but locally for QA you also want to make sure you are installing tor from tor-test-apt.freedom.press

Checklist

If you made changes to the system configuration:

Runing tests locally now, I had to make a tweak to the tor apt test so it doesnt run in the release* branches (same behavior as when this tor apt test is supposed to run).

@msheiny msheiny requested a review from a user January 26, 2018 22:17
@msheiny msheiny force-pushed the release/apt-test branch 2 times, most recently from cb03649 to e4e12a5 Compare January 29, 2018 21:06
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (release/0.5.2@f898877). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             release/0.5.2    #2941   +/-   ##
================================================
  Coverage                 ?   85.52%           
================================================
  Files                    ?       31           
  Lines                    ?     1914           
  Branches                 ?      213           
================================================
  Hits                     ?     1637           
  Misses                   ?      228           
  Partials                 ?       49

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 f898877...1ddfba2. Read the comment docs.

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.

Thanks @msheiny - confirmed that for our staging-test-with-rebase CI job the .tor_version artifact is stored and is 0.3.2.9-1~trusty+1, and that test_tor_mirror_present is passing in local staging

@@ -9,6 +9,14 @@
# WHEN REINSTATING REBOOT
- include: reboot_and_wait.yml
when: "false"
- include: tor_apt_test.yml
when: (lookup('env','CIRCLE_BRANCH')|default('na')).startswith('release')
Copy link
Contributor

Choose a reason for hiding this comment

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

@msheiny Assigning defaults to env lookups often leads to surprises. I believe it's currently not possible to override an undefined env var with |default, but the lookup will return an empty string, which will satisfies the conditional logic here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohhh shit you are totally right @conorsch - thanks for pointing that out

Copy link
Contributor

Choose a reason for hiding this comment

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

Older versions of Ansible threw an uncatchable exception in the event of an undefined env var lookup, so the behavior has improved somewhat, but the silent ignore of |default warrants careful attention. Still, good to go here. 👌

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

LGTM. Note this will run the apt-test logic only on release* branches, not on develop. That's fine, just being super clear about the expected changes here.

@redshiftzero redshiftzero merged commit e5bd387 into release/0.5.2 Jan 30, 2018
@redshiftzero redshiftzero added this to the 0.5.2 milestone Feb 3, 2018
redshiftzero added a commit that referenced this pull request Feb 3, 2018
Conflicts:
	.circleci/config.yml

Favored develop since admin test jobs were added in develop in #2758.

	install_files/ansible-base/roles/ossec/files/test_admin_key.pub
	install_files/ansible-base/roles/ossec/files/test_admin_key.sec

Favored develop for these changes since these keys in 0.5.2 were
erroneously both public keys (fixed in #2925).

	install_files/ansible-base/securedrop-configure.yml

Deleted this file as it was removed in develop during the sdconfig
refactor (#2758) from Ansible to Python. The locale prompt additions added
in SecureDrop 0.5.2 were added in #2758 on develop.

	molecule/aws/scripts/app-tests.sh

Favored develop since the addition of RTL language testing was
added in #2930.

	molecule/aws/side_effect.yml

Favored release/0.5.2 as these changes were due to the addition
of Tor apt repo testing in CI against release branches (#2941).

	securedrop/Dockerfile

Favored develop since all these gettext commands being merged into
one RUN command was done in #2822 and is still on develop.
redshiftzero added a commit that referenced this pull request Feb 4, 2018
Conflicts:
	.circleci/config.yml

Favored develop since admin test jobs were added in develop in #2758.

	install_files/ansible-base/roles/ossec/files/test_admin_key.pub
	install_files/ansible-base/roles/ossec/files/test_admin_key.sec

Favored develop for these changes since these keys in 0.5.2 were
erroneously both public keys (fixed in #2925).

	install_files/ansible-base/securedrop-configure.yml

Deleted this file as it was removed in develop during the sdconfig
refactor (#2758) from Ansible to Python. The locale prompt additions added
in SecureDrop 0.5.2 were added in #2758 on develop.

	molecule/aws/scripts/app-tests.sh

Favored develop since the addition of RTL language testing was
added in #2930.

	molecule/aws/side_effect.yml

Favored release/0.5.2 as these changes were due to the addition
of Tor apt repo testing in CI against release branches (#2941).

	securedrop/Dockerfile

Favored develop since all these gettext commands being merged into
one RUN command was done in #2822 and is still on develop.
redshiftzero added a commit that referenced this pull request Feb 5, 2018
Conflicts:
	.circleci/config.yml

Favored develop since admin test jobs were added in develop in #2758.

	install_files/ansible-base/roles/ossec/files/test_admin_key.pub
	install_files/ansible-base/roles/ossec/files/test_admin_key.sec

Favored develop for these changes since these keys in 0.5.2 were
erroneously both public keys (fixed in #2925).

	install_files/ansible-base/securedrop-configure.yml

Deleted this file as it was removed in develop during the sdconfig
refactor (#2758) from Ansible to Python. The locale prompt additions added
in SecureDrop 0.5.2 were added in #2758 on develop.

	molecule/aws/scripts/app-tests.sh

Favored develop since the addition of RTL language testing was
added in #2930.

	molecule/aws/side_effect.yml

Favored release/0.5.2 as these changes were due to the addition
of Tor apt repo testing in CI against release branches (#2941).

	securedrop/Dockerfile

Favored develop since all these gettext commands being merged into
one RUN command was done in #2822 and is still on develop.

	docs/development/contributor_guidelines.rst

Favored develop since these contributor guidelines were added recently in #2972.
redshiftzero added a commit that referenced this pull request Feb 6, 2018
Conflicts:
	.circleci/config.yml

Favored develop since admin test jobs were added in develop in #2758.

	install_files/ansible-base/roles/ossec/files/test_admin_key.pub
	install_files/ansible-base/roles/ossec/files/test_admin_key.sec

Favored develop for these changes since these keys in 0.5.2 were
erroneously both public keys (fixed in #2925).

	install_files/ansible-base/securedrop-configure.yml

Deleted this file as it was removed in develop during the sdconfig
refactor (#2758) from Ansible to Python. The locale prompt additions added
in SecureDrop 0.5.2 were added in #2758 on develop.

	molecule/aws/scripts/app-tests.sh

Favored develop since the addition of RTL language testing was
added in #2930.

	molecule/aws/side_effect.yml

Favored release/0.5.2 as these changes were due to the addition
of Tor apt repo testing in CI against release branches (#2941).

	securedrop/Dockerfile

Favored develop since all these gettext commands being merged into
one RUN command was done in #2822 and is still on develop.

	docs/development/contributor_guidelines.rst

Favored develop since these contributor guidelines were added recently in #2972.
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.

None yet

4 participants