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

[pilot] app_version and app_build should not be fetched from a local IPA or PKG when distribute_only is set #20488

Merged
merged 2 commits into from
Jul 22, 2022

Conversation

tremblay
Copy link
Contributor

@tremblay tremblay commented Jul 20, 2022

🔑

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Resolves #20479

Description

As mentioned in the title and the linked issue, a local IPA or PKG should not override the info provided to the command if distribute_only is set.

My particular test case used a repo with an iOS and macOS target, and an older macOS .pkg file sitting in the repo. When I ran the command from the linked issue, I ended up waiting for an iOS build with the version number on the old macOS .pkg, even though I passed the correct build_number to the command. With this change, the distribute_only argument will cause the code to fall through to the step where build_number is picked up from the config.

Testing Steps

@tremblay
Copy link
Contributor Author

Unfortunately, I'm not seeing what I've done wrong with regards to the CircleCI checks

@getaaron
Copy link
Collaborator

@tremblay can you please connect circleci to your github account and rerun the build? once you've logged into CircleCI using your GitHub account, you should be able to rerun this branch from the CircleCI UI (or just push an empty commit to this branch if that's easier)

Copy link
Collaborator

@getaaron getaaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be helpful to add a test, you could basically copy this one:

it "wait given :pkg" do
options = { pkg: "some_path.pkg" }
fake_build_manager.instance_variable_set(:@config, options)
expect(fake_build_manager).to receive(:fetch_app_platform)
expect(FastlaneCore::IpaFileAnalyser).to_not(receive(:fetch_app_version))
expect(FastlaneCore::IpaFileAnalyser).to_not(receive(:fetch_app_build))
expect(FastlaneCore::PkgFileAnalyser).to receive(:fetch_app_version).and_return("1.2.3")
expect(FastlaneCore::PkgFileAnalyser).to receive(:fetch_app_build).and_return("123")
fake_build = double
expect(fake_build).to receive(:app_version).and_return("1.2.3")
expect(fake_build).to receive(:version).and_return("123")
expect(FastlaneCore::BuildWatcher).to receive(:wait_for_build_processing_to_be_complete).and_return(fake_build)
fake_build_manager.wait_for_build_processing_to_be_complete
end

but pass in a pkg, build number, and distribute_only, then expect that it uses the number you pass in and it does not call PkgFileAnalyser

WDYT?

@tremblay
Copy link
Contributor Author

@getaaron thanks for the help! CircleCI is now running, and I added a test like you suggested. I also noticed that there were 2 copies of the exact same test wait given :app_version and :build_number, so I removed the 2nd one. Hope that's alright!

Copy link
Collaborator

@getaaron getaaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks so much for your work on this! 🚀🚀🚀

@getaaron getaaron changed the title app_version and app_build should not be fetched from a local IPA or PKG when distribute_only is set [pilot] app_version and app_build should not be fetched from a local IPA or PKG when distribute_only is set Jul 22, 2022
@getaaron getaaron merged commit f221c95 into fastlane:master Jul 22, 2022
@fastlane-bot
Copy link

Hey @tremblay 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

@fastlane-bot
Copy link

Congratulations! 🎉 This was released as part of fastlane 2.209.0 🚀

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

Successfully merging this pull request may close these issues.

upload_to_testflight ignores build_number and uses the latest Mac build instead of the latest iOS
3 participants