[GitHub Actions] Fix logic that was incorrectly skipping tests

[michaeljmarshall]

Motivation

Currently, when there are changes made to code and documentation in the same PR, many of the tests are skipped because the conditional logic is not being parsed as expected.

@jerrypeng noticed this issue on this PR: #10254.

Modifications

Fix the usage of ${{}}.

Verifying this change

I ran tests in my personal GitHub repository to validate this change. Here is the PR that shows it: https://github.com/michaeljmarshall/pulsar/pull/2/checks. I used echo to validate what changed. (Note that the PR uses the base of #10254 to reproduce the problem.)

Does this pull request potentially affect one of the following parts:

This affects all tests that run for PRs that include changes to documentation and code.

[michaeljmarshall]

@lhotari, @sijie, @eolivelli - PTAL. This is an urgent fix that should be merged as soon as we're good with it.

[lhotari]

Thank you @michaeljmarshall for investigating and debugging this. I'll review this soon.

[lhotari]

LGTM. Good work @michaeljmarshall!

[michaeljmarshall]

@lhotari thanks!

I added comments to this staging PR michaeljmarshall#2 to show where and how I tested the changes.

I haven't been able to find explicit documentation on this specific case of interpolation yet. However, it does seem that this change produces the expected outcome for the affected tests.

[eolivelli]

Lgtm

@codelipenghui @congbobo184 ptal

[lhotari]

I added comments to this staging PR michaeljmarshall#2 to show where and how I tested the changes.

Yes, really good. I checked that while reviewing.

I haven't been able to find explicit documentation on this specific case of interpolation yet. However, it does seem that this change produces the expected outcome for the affected tests.

The documentation is here: https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#about-contexts-and-expressions . However there isn't a clear answer.
What seems to happen is that when using this syntax:

if: ${{ steps.changes.outputs.all_count > steps.changes.outputs.docs_count }}

the values are compared as string values and not numeric values.
In the example case, "40" > "7" is false.

When using the other syntax, the explicit expressions ${{ .... }} will trigger the autodetection of the value. This converts them into numbers

if: ${{ steps.changes.outputs.all_count }} > ${{ steps.changes.outputs.docs_count }}

Now 40 > 7 is true.

There is no documentation about how to explicitly convert value to a number. I believe that your fix is the correct way to do it. Thanks for saving my day @michaeljmarshall !

[lhotari]

There is no documentation about how to explicitly convert value to a number. I believe that your fix is the correct way to do it.

I found the documentation. 'Literals': https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#literals . The fix is the correct way to convert to numbers for comparison.

[michaeljmarshall]

@lhotari thank you for finding and sharing the documentation!

[lhotari]

Just sharing more from the documentation: It that seems fromJSON function would also be a way to parse a number in a string as a number.

something like this could possibly work too.

if: ${{ fromJSON(steps.changes.outputs.all_count) > fromJSON(steps.changes.outputs.docs_count) }}

(I think this is very verbose compared to the solution in this PR)

fromJSON("40") > fromJSON("7") -> 40 > 7 -> true