actions / toolkit

The GitHub ToolKit for developing GitHub Actions.
https://github.com/features/actions
MIT License
5k stars 1.44k forks source link

`getBooleanInput` ignores `options.required` #844

Open RA80533 opened 3 years ago

RA80533 commented 3 years ago

Describe the bug

  1. Calling getBooleanInput to read an unset input
  2. Calling getBooleanInput with { required: false } to read an unset input throws an error

Expected behavior getBooleanInput should function the same as getInput in the following ways:

rethab commented 2 years ago

For future visitors, this issue was discussed when the getBooleanInput function was added and it was decided against (signed off by @thboop):

the function is used to deal with boolean input, It should have a default value in action.yml if it is not necessary

ref: https://github.com/actions/toolkit/pull/725#issuecomment-828120911

However, imo since this function does accept a flag required, it is very surprising that this is not respected. Defaulting to false seems like a sensible default to me.

I'd be happy to contribute a PR for this if you agree @thboop

yi-Xu-0100 commented 2 years ago

In the getBooleanInput function, it uses getInput to get the value from action. The required: false will be dealt with the getInput and return the default value. 👀

Could you provide a simple reproducing repository?

RomainMuller commented 2 years ago

actions/setup-dotnet@v2 used to work without passing include-prerelease to it until yesterday...

Today, I get this:

Run actions/setup-dotnet@v2
  with:
    dotnet-version: 3.1.x
  env:
    DOTNET_NOLOGO: true
    NODE_OPTIONS: --max-old-space-size=4096
Error: Input does not meet YAML 1.2 "Core Schema" specification: include-prerelease
Support boolean input list: `true | True | TRUE | false | False | FALSE`

This appears to have been broken recently (although it's not clear to me how actions/setup-dotnet updates their dependency on actions/tooklit).

According to the code, the actions/setup-dotnet@v2 action actually has a default of false since 2 months ago (so setting a default does not save you here): https://github.com/actions/setup-dotnet/blob/a351d9ea84bc76ec7508debf02a39d88f8b6c0c0/action.yml#L21


Update: I just realized my actions/setup-dotnet had the following clause:

include-prerelease: ${{ matrix.dotnet-prerelease }}

While there was no setting for dotnet-prerelease in the matrix, resulting in the value being explicitly undefined. This might be the cause of the problem I'm observing (the behavior nevertheless changed very recently, but I'm willing to consider it is my fault and this only accidentally worked before).

Relequestual commented 1 year ago

This is also problematic when trying to run tests for your action, as you have to manually set inputs via envrionment. I've defined a default in action.yaml, but it has no impact when running tests.

Vampire commented 1 year ago

That has nothing to do with the issue here which is about "required" being ignored, does it?

Relequestual commented 1 year ago

That has nothing to do with the issue here which is about "required" being ignored, does it?

I think it does actually. The argument above was saying you could provide a default in the YAML for the action, but you can't do that when the YAML isn't being used.

I was providing a different use case where the workaround isn't valid.

LittleColin commented 1 month ago

I see this issue is now pretty old and no one is addressing it - this is a particularly annoying one for us as we are using two approaches to check our action changes aren't going to break CIs and neither picked up on this because:

  1. https://github.com/rhysd/actionlint correctly doesn't mind an optional boolean parameter without a default
  2. unit tests mocked out the getBooleanInput so masked the problem

So now we're going to write another "validator" because this hasn't been fixed to ensure that all boolean inputs have a default value if not required.