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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: always use LocalOnlyGitRepo source with dry_run command #1452

Merged
merged 1 commit into from Feb 4, 2024

Conversation

imaginaris
Copy link
Contributor

This PR should fix an issue that happens in my project.

We are running danger dry_run on master branch (no PR) on Bitrise CI machines.
After upgrading ruby version from 2.7.6 to 3.1.4 we started to get following error:
/Users/vagrant/.asdf/installs/ruby/3.1.4/lib/ruby/gems/3.1.0/gems/danger-9.3.1/lib/danger/request_sources/bitbucket_cloud_api.rb:125:in fetch_json': Credentials not available. Provide DANGER_BITBUCKETCLOUD_USERNAME, DANGER_BITBUCKETCLOUD_UUID, and DANGER_BITBUCKETCLOUD_PASSWORD as environment variables. (RuntimeError)`

Even though we use Github.com repos and the same Danger version (9.3.1) as before.

The CI.available_ci_sources is a set and local_ci_source method in environment_manager.rb uses a find function to search for the first valid source in that set. A set has undefined order of elements when treated as a list so when there are multiple CI sources valid (in my case Bitrise and LocalOnly), the local_ci_source method (theoretically) can return a different value each time.
Returning Danger::Bitrise value in my case can't work because there is no PR info in the system env when I'm running danger dry_run on no-PR build. (Bitrise class tries to init all RequestSource classes from supported_request_sources one by one).

Long story short, when running danger dry_run only LocalOnlyGitRepo should be used as a CI source.
I did my best in this PR (I'm not a ruby dev 馃檪) so any comments are welcome.

@manicmaniac
Copy link
Member

@imaginaris
Thanks you for the detailed description. The change looks reasonable to me.
Could you rebase it on master and resolve conflicts?

@imaginaris
Copy link
Contributor Author

@manicmaniac Done.

@manicmaniac
Copy link
Member

@imaginaris Could you resolve the following lint error?

https://github.com/danger/danger/actions/runs/7768462603/job/21188021833?pr=1452

Run bundle exec rubocop
Inspecting 219 files
.........................................................................C.................................................................................................................................................

Offenses:

lib/danger/danger_core/environment_manager.rb:10:[7](https://github.com/danger/danger/actions/runs/7768462603/job/21188021833?pr=1452#step:5:8): C: [Correctable] Layout/EmptyLineAfterGuardClause: Add empty line after guard clause.
      return Danger::LocalOnlyGitRepo if LocalOnlyGitRepo.validates_as_ci? env
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

21[9](https://github.com/danger/danger/actions/runs/7768462603/job/21188021833?pr=1452#step:5:10) files inspected, 1 offense detected, 1 offense autocorrectable

@imaginaris
Copy link
Contributor Author

@manicmaniac Fixed. The CI job should pass now.

@manicmaniac manicmaniac merged commit 105e2de into danger:master Feb 4, 2024
12 checks passed
@imaginaris imaginaris deleted the fix/dry_run-on-ci branch February 5, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants