This fixes an issue with the annotate_test_failures utility when the BUILDKITE_LABEL happens to contains special characters, like is the case for example in Tumblr-Android with steps named E2E tests #1 and E2E tests #2.
(†) That hypothesis (about why those "E2E tests #1" / "E2E tests #2" annotations were never removed on step retry in Tumblr because of that # character messing things up) was confirmed via this commit and this CI build, by creating two annotations, one with a regular context string and one with a tricky one, and showing that the regular one was removed as expected but the tricky one failed to be removed due to its # character in the context string.
How?
Use the BUILDKITE_STEP_ID instead of BUILDKITE_LABEL to construct the annotation context string. That env var not only is an UUID and thus guaranteed to not have any special characters, but also keeps the same value across multiple retries of the same step (as opposed to BUILDKITE_JOB_ID which is a new UUID on every retry)
Use .downcase.gsub(/\W/, '-') to "slug-ify" the context string, ensuring any character other than [a-z0-9_] are turned into a -. This is still needed despite BUILDKITE_STEP_ID being a UUID without special characters, because the string we build to use for the annotation context must also include the value of state variable ("have failed" / "were flaky") to allow one step to generate multiple annotations for reporting different states and types of test failures/warnings.
[x] I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.
cc @adimoldovan FYI as to why the Buildkite annotations were never removed like they were expected to after a successful retry of the "E2E tests #N" steps on Tumblr-Android.
Why?
This fixes an issue with the
annotate_test_failuresutility when theBUILDKITE_LABELhappens to contains special characters, like is the case for example in Tumblr-Android with steps namedE2E tests #1andE2E tests #2.Those
#in the label means that the string we derive from it to use as the annotation context end up messing things up, because that#is not escaped, and when we try tobuildkite-agent annotation remove --context 'E2E tests #1', this fails to find the annotation and to remove it, because of that special character confusing the Buildkite agent API(†).(†) That hypothesis (about why those "E2E tests #1" / "E2E tests #2" annotations were never removed on step retry in Tumblr because of that
#character messing things up) was confirmed via this commit and this CI build, by creating two annotations, one with a regular context string and one with a tricky one, and showing that the regular one was removed as expected but the tricky one failed to be removed due to its#character in the context string.How?
BUILDKITE_STEP_IDinstead ofBUILDKITE_LABELto construct the annotation context string. That env var not only is an UUID and thus guaranteed to not have any special characters, but also keeps the same value across multiple retries of the same step (as opposed toBUILDKITE_JOB_IDwhich is a new UUID on every retry).downcase.gsub(/\W/, '-')to "slug-ify" the context string, ensuring any character other than[a-z0-9_]are turned into a-. This is still needed despiteBUILDKITE_STEP_IDbeing a UUID without special characters, because the string we build to use for the annotation context must also include the value ofstatevariable ("have failed"/"were flaky") to allow one step to generate multiple annotations for reporting different states and types of test failures/warnings.CHANGELOG.mdif necessary.cc @adimoldovan FYI as to why the Buildkite annotations were never removed like they were expected to after a successful retry of the "E2E tests #N" steps on Tumblr-Android.