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
This commit is contained in:
George Kalpakas 2019-11-22 15:16:39 +02:00 committed by Matias Niemelä
parent 9bc8864d03
commit 9b5299131b
3 changed files with 12 additions and 40 deletions

View File

@ -12,4 +12,4 @@ source ../scripts/ci/payload-size.sh
# Provide node_modules from aio # Provide node_modules from aio
NODE_MODULES_BIN=$PROJECT_ROOT/aio/node_modules/.bin/ 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"

View File

@ -64,7 +64,7 @@ for testDir in ${TEST_DIRS}; do
yarn build yarn build
fi fi
trackPayloadSize "$testDir" "dist/*.js" true false "${basedir}/integration/_payload-limits.json" trackPayloadSize "$testDir" "dist/*.js" true "${basedir}/integration/_payload-limits.json"
fi fi
# remove the temporary node modules directory to keep the source folder clean. # remove the temporary node modules directory to keep the source folder clean.
@ -73,5 +73,5 @@ for testDir in ${TEST_DIRS}; do
done done
if $CI; then 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 fi

View File

@ -80,36 +80,6 @@ addMessage() {
payloadData="$payloadData\"message\": \"$message\", " 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 `<SHA-1>...<SHA-2>` 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. # Convert the current `payloadData` value to a JSON string.
# (Basically remove trailing `,` and wrap in `{...}`.) # (Basically remove trailing `,` and wrap in `{...}`.)
payloadToJson() { payloadToJson() {
@ -134,18 +104,17 @@ uploadData() {
# $1: string - The name in database. # $1: string - The name in database.
# $2: string - The file path. # $2: string - The file path.
# $3: true | false - Whether to check the payload size and fail the test if it exceeds limit. # $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. # $4: [string] - The payload size limit file. Only necessary if `$3` is `true`.
# $5: [string] - The payload size limit file. Only necessary if `$3` is `true`.
trackPayloadSize() { trackPayloadSize() {
name="$1" name="$1"
path="$2" path="$2"
checkSize="$3" checkSize="$3"
trackChangeType="$4" limitFile="${4:-}"
limitFile="${5:-}"
payloadData="" payloadData=""
# Calculate the file sizes. # Calculate the file sizes.
echo "Calculating sizes for files in '$path'..."
for filename in $path; do for filename in $path; do
calculateSize calculateSize
done done
@ -155,17 +124,20 @@ trackPayloadSize() {
# If this is a non-PR build, upload the data to firebase. # If this is a non-PR build, upload the data to firebase.
if [[ "$CI_PULL_REQUEST" == "false" ]]; then if [[ "$CI_PULL_REQUEST" == "false" ]]; then
if [[ $trackChangeType = true ]]; then echo "Uploading data for '$name'..."
addChangeType $CI_COMMIT_RANGE
fi
addTimestamp addTimestamp
addBuildUrl $CI_BUILD_URL addBuildUrl $CI_BUILD_URL
addMessage $CI_COMMIT_RANGE addMessage $CI_COMMIT_RANGE
uploadData $name uploadData $name
else
echo "Skipped uploading data for '$name', because this is a pull request."
fi fi
# Check the file sizes against the specified limits. # Check the file sizes against the specified limits.
if [[ $checkSize = true ]]; then if [[ $checkSize = true ]]; then
echo "Verifying sizes against '$limitFile'..."
checkSize $name $limitFile checkSize $name $limitFile
else
echo "Skipped verifying sizes (checkSize: false)."
fi fi
} }