Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing a String including a comma doesn't fail in validation on an option that expects Array #20745

Open
4 tasks done
ainame opened this issue Oct 14, 2022 · 2 comments
Open
4 tasks done

Comments

@ainame
Copy link
Contributor

ainame commented Oct 14, 2022

New Issue Checklist

Issue Description

I had the same issue as #7283. I used pilot and gave a group name Staff, Family & Friends to it.

upload_to_testflight(
   groups: "Staff, Family & Friends",
   distribute_external: true,
   // ...
)

I understand this is my mistake passing a String to the option expecting Array. However, this should have raised an error based on the validation defined on the option. There is a code to check if the given value is Array.

FastlaneCore::ConfigItem.new(key: :groups,
short_option: "-g",
env_name: "PILOT_GROUPS",
description: "Associate tester to one group or more by group name / group id. E.g. `-g \"Team 1\",\"Team 2\"` This is required when `distribute_external` option is set to true or when we want to add a tester to one or more external testing groups ",
optional: true,
type: Array,
verify_block: proc do |value|
UI.user_error!("Could not evaluate array from '#{value}'") unless value.kind_of?(Array)
end),

This didn't work because fastlane does convert input value based on each option's type info and then calls the validation. In my case, my input value as a string literal "Staff, Family & Friends" was first split into Array and then passed the validation.

def verify_value_exists
# Make sure the given value keys exist
@values.each do |key, value|
next if key == :trace # special treatment
option = self.verify_options_key!(key)
@values[key] = option.auto_convert_value(value)
UI.deprecated("Using deprecated option: '--#{key}' (#{option.deprecated})") if option.deprecated
option.verify!(@values[key]) # Call the verify block for it too
end
end

I'd say I expect that fastlane will fail with it. I'll explore if we can improve this 🙏

Complete output when running fastlane, including the stack trace and command used

There's no useful information as upload_to_testflight just succeeded but it didn't assign a tester group to the build.

Environment

✅ fastlane environment ✅

fastlane 2.210.1

Stack

Key Value
OS 12.6
Ruby 3.0.2
Bundler? true
Git git version 2.38.0
Installation Source ~/.rbenv/versions/3.0.2/lib/ruby/gems/3.0.0/bin/fastlane
Host macOS 12.6 (21G115)
Ruby Lib Dir ~/.rbenv/versions/3.0.2/lib
OpenSSL Version OpenSSL 1.1.1l 24 Aug 2021
Is contained false
Is homebrew false
Is installed via Fabric.app false
Xcode Path /Applications/Xcode_13.4.1.app/Contents/Developer/
Xcode Version 13.4.1
Swift Version 5.6.1

System Locale

Variable Value
LANG en_GB.UTF-8
LC_ALL
LANGUAGE
@ainame ainame self-assigned this Oct 14, 2022
@fastlane-bot
Copy link

It seems like you have not included the output of fastlane env
To make it easier for us help you resolve this issue, please update the issue to include the output of fastlane env 👍

@ainame ainame changed the title Passing a String including a comma doesn't fail validation on an option that expects Array Passing a String including a comma doesn't fail in validation on an option that expects Array Oct 14, 2022
@fastlane-bot
Copy link

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates.

Please make sure to update to the latest fastlane version and check if that solves the issue. Let us know if that works for you by adding a comment 👍

Friendly reminder: contributions are always welcome! Check out CONTRIBUTING.md for more information on how to help with fastlane and feel free to tackle this issue yourself 💪

This issue will be auto-closed if there is no reply within 1 month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants