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

[WIP] SPM testing! (+ some other minor updates) #1896

Merged
merged 10 commits into from Sep 2, 2019

Conversation

@sunshinejr
Copy link
Member

commented Aug 13, 2019

Thanks to @jeffctown, OHHTTPStubs is getting a support for SPM! This was something that was keeping us from adopting SPM testing. It's still in PR so not merged and so this PR is in [WIP] state.

@sunshinejr sunshinejr referenced this pull request Aug 13, 2019
3 of 3 tasks complete
@f-meloni

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Skipping Danger due to this run not executing on a PR.

😮 what happened to Danger js?

@sunshinejr

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

@f-meloni how did you catch that? 😄 Not sure what happened, but it seems like for some reason there is no source (doubt the isPr is set to false, but maybe)?

@f-meloni

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Was having a look to see if a cache reset was still needed (and I think it was, but it didn't arrive to that point :D)
It looks everything ok in that build, the only thing I can think is that CIRCLE_CI_API_TOKEN is missing for any reason https://github.com/danger/danger-js/blob/master/source/ci_source/providers/Circle.ts#L62? 🤔

@sunshinejr

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

Interesting! I'm pretty sure we didn't change anything in the CircleCI settings since #1891 🤔

@f-meloni

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

Didn't see that PR, it looks that solved the cache problem for Danger-swift as well (pretty cool :P)
The only thing I can think is that, given the other values are almost always there, it recognises is in CI and Circle recognises is running on PR, then should have all the requested PR values like CIRCLE_PR_NUMBER, the only thing I could think of is a problem with the token :/

@sunshinejr

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

Ahh, @f-meloni, I think this might be the case:

A common scenario is when CircleCI begins building a commit before the commit becomes associated with a PR (e.g. a developer pushes their branch to the remote repo for the first time. CircleCI spins up and begins building. Moments later the developer creates a PR on GitHub. Since the build process started before the PR existed, Danger won't be able to use the Circle-provided environment variables to retrieve PR metadata.)

I turned the Only build pull requests flag to avoid this issue for now.

@sunshinejr

This comment has been minimized.

Copy link
Member Author

commented Sep 2, 2019

Although this is still in WIP process, I'm gonna merge it as this has some value in it (SPM fixes, version bump for ReactiveSwift 6.1). If the PR on OHHTTPStubs is updated, I will make another PR with a follow-up as well.

@sunshinejr sunshinejr merged commit a145cf3 into development Sep 2, 2019

2 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: carthage_without_swiftlint_integration Your tests passed on CircleCI!
Details

@sunshinejr sunshinejr deleted the feature/spm_tests branch Sep 2, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Sep 2, 2019

Codecov Report

Merging #1896 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #1896   +/-   ##
============================================
  Coverage        92.27%   92.27%           
============================================
  Files               26       26           
  Lines              919      919           
============================================
  Hits               848      848           
  Misses              71       71
Impacted Files Coverage Δ
Sources/Moya/RequestTypeWrapper.swift 76.92% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94c43d7...b465d17. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.