From 9b5299131b83417f5182bd0389e5a433d91221a2 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 22 Nov 2019 15:16:39 +0200 Subject: [PATCH] ci: remove change type from uploaded payload size data (#33987) The change type was only recorded for `aio/` and was not correct anyway. For example: - It considered `package.json` changes as `application` (even if only `package.json` and `yarn.lock` had changed). - It failed to account for changes in `@angular/*` dependencies, when using the locally built Angular packages (instead reporting them as `other`). - It only looked at the last commit, so it failed to provide accurate information for multi-commit builds (which are rare, but possible). For the above reasons (and because there is no straight-forward way of fixing it), this commit removes the change type from the uploaded data. If necessary, it is still possible to find the type of changes from the uploaded info (e.g. extract the associated commits and look at their changes using git). PR Close #33987 --- aio/scripts/payload.sh | 2 +- integration/run_tests.sh | 4 ++-- scripts/ci/payload-size.sh | 46 ++++++++------------------------------ 3 files changed, 12 insertions(+), 40 deletions(-) diff --git a/aio/scripts/payload.sh b/aio/scripts/payload.sh index 72ee70a636..51606ba9f4 100755 --- a/aio/scripts/payload.sh +++ b/aio/scripts/payload.sh @@ -12,4 +12,4 @@ source ../scripts/ci/payload-size.sh # Provide node_modules from aio NODE_MODULES_BIN=$PROJECT_ROOT/aio/node_modules/.bin/ -trackPayloadSize "$target" "dist/*.js" true true "${thisDir}/_payload-limits.json" +trackPayloadSize "$target" "dist/*.js" true "${thisDir}/_payload-limits.json" diff --git a/integration/run_tests.sh b/integration/run_tests.sh index 9ba43feb66..181c8e5af1 100755 --- a/integration/run_tests.sh +++ b/integration/run_tests.sh @@ -64,7 +64,7 @@ for testDir in ${TEST_DIRS}; do yarn build fi - trackPayloadSize "$testDir" "dist/*.js" true false "${basedir}/integration/_payload-limits.json" + trackPayloadSize "$testDir" "dist/*.js" true "${basedir}/integration/_payload-limits.json" fi # remove the temporary node modules directory to keep the source folder clean. @@ -73,5 +73,5 @@ for testDir in ${TEST_DIRS}; do done if $CI; then - trackPayloadSize "umd" "../dist/packages-dist/*/bundles/*.umd.min.js" false false + trackPayloadSize "umd" "../dist/packages-dist/*/bundles/*.umd.min.js" false fi diff --git a/scripts/ci/payload-size.sh b/scripts/ci/payload-size.sh index 3efd4717a7..38caeb1e88 100644 --- a/scripts/ci/payload-size.sh +++ b/scripts/ci/payload-size.sh @@ -80,36 +80,6 @@ addMessage() { payloadData="$payloadData\"message\": \"$message\", " } -# Add change source: `application`, `dependencies`, or `application+dependencies` -# Read from global variable `$parentDir`. -# Update the change source in global variable `$payloadData`. -# $1: string - The commit range for this build (in `...` format). -addChangeType() { - commitRange="$1" - - yarnChanged=false - allChangedFiles=$(git diff --name-only $commitRange $parentDir | wc -l) - allChangedFileNames=$(git diff --name-only $commitRange $parentDir) - - if [[ $allChangedFileNames == *"yarn.lock"* ]]; then - yarnChanged=true - fi - - if [[ $allChangedFiles -eq 1 ]] && [[ "$yarnChanged" = true ]]; then - # only yarn.lock changed - change='dependencies' - elif [[ $allChangedFiles -gt 1 ]] && [[ "$yarnChanged" = true ]]; then - change='application+dependencies' - elif [[ $allChangedFiles -gt 0 ]]; then - change='application' - else - # Nothing changed inside $parentDir (but size may still be affected; e.g. when using the locally - # built packages) - change='other' - fi - payloadData="$payloadData\"change\": \"$change\", " -} - # Convert the current `payloadData` value to a JSON string. # (Basically remove trailing `,` and wrap in `{...}`.) payloadToJson() { @@ -134,18 +104,17 @@ uploadData() { # $1: string - The name in database. # $2: string - The file path. # $3: true | false - Whether to check the payload size and fail the test if it exceeds limit. -# $4: true | false - Whether to record the type of changes. -# $5: [string] - The payload size limit file. Only necessary if `$3` is `true`. +# $4: [string] - The payload size limit file. Only necessary if `$3` is `true`. trackPayloadSize() { name="$1" path="$2" checkSize="$3" - trackChangeType="$4" - limitFile="${5:-}" + limitFile="${4:-}" payloadData="" # Calculate the file sizes. + echo "Calculating sizes for files in '$path'..." for filename in $path; do calculateSize done @@ -155,17 +124,20 @@ trackPayloadSize() { # If this is a non-PR build, upload the data to firebase. if [[ "$CI_PULL_REQUEST" == "false" ]]; then - if [[ $trackChangeType = true ]]; then - addChangeType $CI_COMMIT_RANGE - fi + echo "Uploading data for '$name'..." addTimestamp addBuildUrl $CI_BUILD_URL addMessage $CI_COMMIT_RANGE uploadData $name + else + echo "Skipped uploading data for '$name', because this is a pull request." fi # Check the file sizes against the specified limits. if [[ $checkSize = true ]]; then + echo "Verifying sizes against '$limitFile'..." checkSize $name $limitFile + else + echo "Skipped verifying sizes (checkSize: false)." fi }