Skip to content

Remove custom check for CI in favor of using Fastlane's one #336

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

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ def initialize(params = {})
end

def source_contents
# TODO: This works only on CircleCI. I chose this variable because it's the one checked by Fastlane::is_ci.
# Fastlane::is_ci doesn't work here, so reimplementing the code has been necessary.
# (This should be updated if we change CI or if fastlane is updated.)
return File.read(secrets_repository_file_path) unless self.encrypt || ENV.key?('CIRCLECI')
return File.read(secrets_repository_file_path) unless self.encrypt || FastlaneCore::Helper.is_ci?
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea why the comment initially said that "Fastlane::is_ci? doesn't work here "?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure the reason is that is_ci is a Fastlane action and can't be called directly.

If we put it in here, we get a no method error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. As opposed to calling the helper directly as you did here 👍👌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See is_ci? source here.

To test this change, I run the tests as usual first, the re-run them for this file with CI=1:

CI=1 bundle exec rspec spec/file_reference_spec.rb

I got the following failure:

  1) Fastlane::Configuration::FileReference without encryption #source_contents gets the contents from the secrets repo
     Failure/Error: expect(subject.source_contents).to eq('source contents')

       expected: "source contents"
            got: nil

       (compared using ==)

That's exactly the failure I would expect to get: is_ci? returned true and we didn't read from the stubbed local secrets but tried to read the encrypted source, which didn't exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

That also means that the test is failing on CI though 😓😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️ is not enough of a reaction...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

image

return nil unless File.file?(encrypted_file_path)

encrypted = File.read(encrypted_file_path)
Expand Down