search

crate-ci / azure-pipelines

Easy continuous integration for Rust projects with Azure Pipelines
MIT License
88 stars 24 forks source link

Workaround for DevOps coercion #50

Closed jonhoo closed 5 years ago

jonhoo commented 5 years ago

The eq and ne functions coerce their second argument to be of the same type as the first: https://docs.microsoft.com/en-us/azure/devops/pipelines/process/expressions#eq. If the first argument is a parameter, and it is set to false or true (as opposed to 'false' or 'true'), then the second argument which we're comparing against will be coerced to a boolean. However, since any non-empty string is coerced to true if you had this expression:

${{ if eq(parameters.foo, 'false') }}:

Then passing foo as true would result in the condition being considered true (since 'false' => true == true), and passing foo as false would result in the condition being considered false by the same reasoning. The exact opposite of what you wanted. See also https://docs.microsoft.com/en-us/azure/devops/pipelines/process/expressions#type-casting

This PR flips the operand order for every invocation of eq and ne such that the arguments are always considered as strings. This fixes the behavior observed above, since false is cast to 'false' and true to 'true'.

It's stupid.

jonhoo commented 5 years ago

We really need a way to test for these things. I almost think we need a step in CI that checks a bunch of properties of the CI jobs run thus far. Not sure how easy that is to do. Something that checks that each and every job was run with the correct arguments and such.

codecov-io commented 5 years ago

Codecov Report

Merging #50 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #50   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines           8      8           
=====================================
  Hits            8      8

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9e8e64f...db26ae1. Read the comment docs.

djc commented 5 years ago

Is there no explicit conversion from string to boolean?

jonhoo commented 5 years ago

There is. Every non-empty string is coerced into true (and that includes the string 'false').