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

[ci] move trusty app tests to nightly job and use circle ci branch filtering logic #4158

Merged
merged 4 commits into from
Feb 22, 2019

Conversation

redshiftzero
Copy link
Contributor

@redshiftzero redshiftzero commented Feb 19, 2019

Status

Ready for review

Description of Changes

This PR makes two improvements to reduce total CI time:

  • Beginning with 0.12.0, news organizations will be migrating to Xenial, and all new installs will be on Xenial. Thus, we should move the Trusty app test job to a nightly job, and monitor it for failures until Trusty is deprecated in the coming months. This reduces total CI time per PR, as the trusty job took 20+ minutes. Related to Make Xenial development/CI environments the default #4155.
  • It turns out that CircleCI enables filtering by branches now, which we can use to replace our custom logic for avoiding unnecessary jobs on docs- branches, currently used for the staging job only. The app-tests were also being run for docs- branches, so we can use the same CircleCI branch filtering logic to easily exclude the staging/application test jobs run for docs- branches. Indeed, previously jobs would be kicked off and then would rapidly pass, but with the CircleCI branch filtering, jobs won't even get kicked off, so this should reduce build pileups. This should also close Docs-only PRs from forks still run staging env in CI #3699.

Note: We'll need to disable the trusty app test job as a required build to merge, holding off on that until we agree

Testing

Verify that the test build here (branch docs-ci-reduce-jobs) only ran two jobs:

screen shot 2019-02-19 at 11 1
0 54 pm

Deployment

CI only

Checklist

CI only

Beginning with 0.12.0, news organizations will be migrating to
Xenial, and all new installs will be on Xenial. Thus, we should
move the Trusty app test job to a nightly job, and monitor it
for failures until Trusty is deprecated in the following months.

This also reduces total CI time per PR, as the trusty job took 20+ minutes.
@redshiftzero redshiftzero changed the title ci: move trusty app tests to nightly job [ci] move trusty app tests to nightly job Feb 20, 2019
@redshiftzero redshiftzero changed the title [ci] move trusty app tests to nightly job [ci] move trusty app tests to nightly job and use circle ci branch filtering logic Feb 20, 2019
@redshiftzero redshiftzero force-pushed the ci-move-trusty-app-test-job-to-nightly branch from 08ba0be to 575ae19 Compare February 20, 2019 07:11
@codecov-io
Copy link

Codecov Report

Merging #4158 into develop will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4158      +/-   ##
===========================================
- Coverage    84.79%   84.72%   -0.08%     
===========================================
  Files           43       43              
  Lines         2782     2782              
  Branches       303      303              
===========================================
- Hits          2359     2357       -2     
- Misses         355      356       +1     
- Partials        68       69       +1
Impacted Files Coverage Δ
securedrop/securedrop/crypto_util.py 94.73% <0%> (-1.76%) ⬇️

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 cb91e59...575ae19. Read the comment docs.

filters:
branches:
ignore:
- /docs-.*/
- static-analysis-and-no-known-cves
- staging-test-with-rebase:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also run Xenial infra tests on each PR instead of the default (Trusty) tests?

@redshiftzero
Copy link
Contributor Author

Once this is approved, I'll remove the requirement for ci/circleci: staging-test-with-rebase and ci/circleci: trusty-app-tests in favor of the corresponding Xenial jobs

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 these changes look good to me. I propose we merge these changes after 0.12.0 release to avoid any side-effects/issues during the QA/release process.

@@ -16,11 +16,6 @@ set -o pipefail
# Assume we're running against Trusty; Xenial also supported.
target_platform="${1:-trusty}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we open a follow-up task to completely eliminate trusty references at some point in the future, and/or replace the defaults with xenial?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so I have task #4155 filed for this which this PR does not close (wanted to get this PR in first so that we can not trigger quite so many builds since we're getting a lot of queueing in circleCI)

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.

LGTM !

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.

Docs-only PRs from forks still run staging env in CI
3 participants