Skip to content

ci: fix the aio_monitoring job (and other minor improvements) #28555

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.

Sign up for GitHub

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

Closed
gkalpak wants to merge 5 commits into from
Closed

ci: fix the aio_monitoring job (and other minor improvements) #28555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f03363
ci(docs-infra): fix `download_yarn` in `aio_monitoring`
gkalpak Feb 6, 2019
85b1d66
refactor: use steps consistently in CircleCI config
gkalpak Feb 6, 2019
2e1cb94
ci: run the `yarn_install` step when necessary
gkalpak Feb 6, 2019
92c970d
ci: save the cache in `build-npm-packages` job instead of `test`
gkalpak Feb 6, 2019
0b9e933
ci(docs-infra): ensure `aio_monitoring` failure notifications do not …
gkalpak Feb 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ var_9: &setup_circleci_bazel_config
var_10: &download_yarn
run:
name: Downloading Yarn
command: |
echo "Attempting to install $CI_YARN_VERSION from https://yarnpkg.com" |
Copy link
Member Author

@gkalpak gkalpak Feb 6, 2019

Choose a reason for hiding this comment

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

This was added for debugging purposes in #28553 and is no longer necessary.
The value of the env var can be seen earlier in the job log (as part of the define_env_vars step and the actualyl installed version can be seen as the output of this step's command.

curl -o- -L https://yarnpkg.com/install.sh | PROFILE=$BASH_ENV bash -s -- --version "$CI_YARN_VERSION"
command: curl -o- -L https://yarnpkg.com/install.sh | PROFILE=$BASH_ENV bash -s -- --version "$CI_YARN_VERSION"

version: 2
jobs:
Expand Down Expand Up @@ -108,9 +106,9 @@ jobs:
- restore_cache:
key: *cache_key
- *define_env_vars
- *setup_circleci_bazel_config
- *download_yarn
- *yarn_install
- *setup_circleci_bazel_config

# Setup remote execution and run RBE-compatible tests.
- *setup_bazel_remote_execution
Expand All @@ -119,12 +117,6 @@ jobs:
- run: sudo cp .circleci/bazel.rc /etc/bazel.bazelrc
- run: yarn bazel test //... --build_tag_filters=-ivy-only,local --test_tag_filters=-ivy-only,local

- save_cache:
key: *cache_key
paths:
- "node_modules"
- "~/bazel_repository_cache"

# Temporary job to test what will happen when we flip the Ivy flag to true
test_ivy_aot:
<<: *job_defaults
Expand All @@ -135,9 +127,9 @@ jobs:
- restore_cache:
key: *cache_key
- *define_env_vars
- *setup_circleci_bazel_config
- *download_yarn
- *yarn_install
- *setup_circleci_bazel_config
- *setup_bazel_remote_execution

# We need to explicitly specify the --symlink_prefix option because otherwise we would
Expand Down Expand Up @@ -280,8 +272,6 @@ jobs:
at: dist
- *define_env_vars
- *download_yarn
# Install root
- *yarn_install
# Install aio
- run: yarn --cwd aio install --frozen-lockfile --non-interactive
# Run examples tests. The "CIRCLE_NODE_INDEX" will be set if "parallelism" is enabled.
Expand All @@ -301,7 +291,6 @@ jobs:
key: *cache_key
- *define_env_vars
- *download_yarn
- *yarn_install
- run: ./aio/scripts/build-artifacts.sh $AIO_SNAPSHOT_ARTIFACT_PATH $CI_PULL_REQUEST $CI_COMMIT
- store_artifacts:
path: *aio_preview_artifact_path
Expand All @@ -323,7 +312,7 @@ jobs:
key: *cache_key
- *define_env_vars
- *download_yarn
- run: yarn install --cwd aio --frozen-lockfile --non-interactive
- run: yarn --cwd aio install --frozen-lockfile --non-interactive
Copy link
Contributor

@petebacondarwin petebacondarwin Feb 6, 2019

Choose a reason for hiding this comment

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

Does this change make any difference?

Copy link
Member Author

@gkalpak gkalpak Feb 6, 2019

Choose a reason for hiding this comment

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

It depends how you look at it. For me it is like:

Previous behavior: Inconsistent config 😞
New Behavior: Consistent config 😄

But that's probably not the answer you are looking for 😁
Here is another one:

It does not make a difference in this case. yarn install --cwd aio and yarn --cwd aio install both run yarn install in the aio/ directory.
BUT...this is not true for custom scripts. For example, if both package.json files have a script foo, then yarn --cwd aio foo will run the foo script from aio/package.json, while yarn foo --cwd aio will run the foo script from ./package.json (and pass --cwd aio as an argument).

So, while it doesn't make a difference here, I think it is safer to always use --cwd aio before the command, so that people might not accidentally copy it, change install to some other script and expect it to work.

Copy link
Contributor

@petebacondarwin petebacondarwin Feb 6, 2019

Choose a reason for hiding this comment

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

Good explanation. Sounds reasonable.

- run:
name: Wait for preview and run tests
command: node aio/scripts/test-preview.js $CI_PULL_REQUEST $CI_COMMIT $CI_AIO_MIN_PWA_SCORE
Expand Down Expand Up @@ -360,6 +349,12 @@ jobs:
paths:
- packages-dist

- save_cache:
key: *cache_key
paths:
- "node_modules"
- "~/bazel_repository_cache"

Copy link
Contributor

@IgorMinar IgorMinar Feb 6, 2019

Choose a reason for hiding this comment

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

awesome!


# Build the ivy npm packages.
build-ivy-npm-packages:
Expand Down Expand Up @@ -410,6 +405,8 @@ jobs:
at: dist
- *define_env_vars
- *download_yarn
# Some integration tests get their dependencies from the root `node_modules/`.
- *yarn_install
# Runs the integration tests in parallel across multiple CircleCI container instances. The
# amount of container nodes for this job is controlled by the "parallelism" option.
- run: ./integration/run_tests.sh ${CIRCLE_NODE_INDEX} ${CIRCLE_NODE_TOTAL}
Expand Down Expand Up @@ -454,14 +451,16 @@ jobs:
<<: *post_checkout
- restore_cache:
key: *cache_key
- *download_yarn
- *define_env_vars
- *download_yarn
- run:
name: Run tests against the deployed apps
command: ./aio/scripts/test-production.sh $CI_AIO_MIN_PWA_SCORE
- run:
name: Notify caretaker about failure
command: 'curl --request POST --header "Content-Type: application/json" --data "{\"text\":\":x: \`$CIRCLE_JOB\` job failed on build $CIRCLE_BUILD_NUM: $CIRCLE_BUILD_URL :scream:\"}" $CI_SECRET_SLACK_CARETAKER_WEBHOOK_URL'
# `$SLACK_CARETAKER_WEBHOOK_URL` is a secret env var defined in CircleCI project settings.
Copy link
Contributor

@IgorMinar IgorMinar Feb 6, 2019

Choose a reason for hiding this comment

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

why not move these docs to script executed by define_env_vars? isn't that the right place for this?

Copy link
Member Author

@gkalpak gkalpak Feb 6, 2019

Choose a reason for hiding this comment

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

I just moved the docs here from that script 😄
This change is about making the notification command not rely on anything from define_env_vars. This way we will still get notified if the define_env_vars step (or an earlier step) fails.

Note: As a general (personal) rule, any script outside the `.circleci/` directory should not rely on CircleCI-specific env vars, but use stuff exported by `env.sh` instead. But it is fine for code inside the `.circleci/` directory (which is by definition CircleCI-specific code) to use CircleCI-specific vars.

# The URL comes from https://angular-team.slack.com/apps/A0F7VRE7N-circleci.
command: 'curl --request POST --header "Content-Type: application/json" --data "{\"text\":\":x: \`$CIRCLE_JOB\` job failed on build $CIRCLE_BUILD_NUM: $CIRCLE_BUILD_URL :scream:\"}" $SLACK_CARETAKER_WEBHOOK_URL'
when: on_fail

legacy-unit-tests-local:
Expand Down
4 changes: 1 addition & 3 deletions .circleci/env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ setPublicVar PROJECT_ROOT "$(pwd)";
setPublicVar CI_AIO_MIN_PWA_SCORE "95";
# This is the branch being built; e.g. `pull/12345` for PR builds.
setPublicVar CI_BRANCH "$CIRCLE_BRANCH";
setPublicVar CI_YARN_VERSION "1.13.0"
# ChromeDriver version compatible with the Chrome version included in the docker image used in
# `.circleci/config.yml`. See http://chromedriver.chromium.org/downloads for a list of versions.
# This variable is intended to be passed as an arg to the `webdriver-manager update` command (e.g.
Expand All @@ -32,15 +31,14 @@ setPublicVar CI_COMMIT_RANGE "`[[ ${CIRCLE_PR_NUMBER:-false} != false ]] && echo
setPublicVar CI_PULL_REQUEST "${CIRCLE_PR_NUMBER:-false}";
setPublicVar CI_REPO_NAME "$CIRCLE_PROJECT_REPONAME";
setPublicVar CI_REPO_OWNER "$CIRCLE_PROJECT_USERNAME";
setPublicVar CI_YARN_VERSION "1.13.0";
Copy link
Member Author

@gkalpak gkalpak Feb 6, 2019

Choose a reason for hiding this comment

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

Yes, alphabetic order is important to me 😇

Copy link
Contributor

@IgorMinar IgorMinar Feb 6, 2019

Choose a reason for hiding this comment

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

more power to you!



####################################################################################################
# Define SECRET environment variables for CircleCI.
####################################################################################################
setSecretVar CI_SECRET_AIO_DEPLOY_FIREBASE_TOKEN "$AIO_DEPLOY_TOKEN";
setSecretVar CI_SECRET_PAYLOAD_FIREBASE_TOKEN "$ANGULAR_PAYLOAD_TOKEN";
# Defined in https://angular-team.slack.com/apps/A0F7VRE7N-circleci.
setSecretVar CI_SECRET_SLACK_CARETAKER_WEBHOOK_URL "$SLACK_CARETAKER_WEBHOOK_URL";


####################################################################################################
Expand Down