search

DiUS / pact-consumer-swift

A Swift / ObjeciveC DSL for creating pacts.
MIT License
98 stars 43 forks source link

Removed Nimble Dependency #87

Closed rubencagnie closed 4 years ago

rubencagnie commented 4 years ago

Gotcha: this is a breaking change, so anyone that is using the MockService initializer without pactVerificationService and errorReporter will fail. They can easily fix this by adding the extensions that can be found in NimbleErrorReporter

TODO - Update documentation before merging if this PR is approved

resolves #57

andrewspinks commented 4 years ago

@rubencagnie thanks heaps for this! I'm still in the midst of reviewing the changes. I'll try to finish it over the weekend.

rubencagnie commented 4 years ago

@andrewspinks - I addressed your comments + replied to some of them

rubencagnie commented 4 years ago

@andrewspinks - I took another look, and found that just using XCTFail should just do the trick. I tested it in the project itself and in a sample project I made. I just pushed my latest code (which is actually mostly deleting files...)

andrewspinks commented 4 years ago

@rubencagnie Apologies, but the build failed with some linting issues:

rubencagnie commented 4 years ago

@andrewspinks - sorry about that. Fixed it and pushed

andrewspinks commented 4 years ago

@rubencagnie build failed again with Build input file cannot be found: 'pact-consumer-swift/Sources/Reporting/XCTestErrorReporter.swift' (in target 'PactConsumerSwift iOS' from project 'PactConsumerSwift')

On travis, the build is run using ./scripts/build.sh

andrewspinks commented 4 years ago

@rubencagnie There were still some build failures for this PR due to a linking issue with XCTest framework on some versions of iOS.

Reason: no suitable image found.  Did find:
    /usr/lib/swift/libswiftXCTest.dylib: mach-o, but not built for iOS simulator

I took a look into it and it appears to be caused by a bug in XCode. Updating the XCode version in the .travis.yml file to xcode11.4 appears to fix the issue. There are also some minor merge conflicts with master.

rubencagnie commented 4 years ago

@andrewspinks - I’m really sorry. Recently changed jobs and with the pandemic (and 2 small children) I just haven’t gotten around to it

andrewspinks commented 4 years ago

@rubencagnie no worries. I have a 9 month old. Getting the time to even read emails can be a challenge!

bethesque commented 4 years ago

Tell me about it.

codecov-commenter commented 4 years ago

Codecov Report

Merging #87 into master will increase coverage by 0.58%. The diff coverage is 88.00%.

@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
+ Coverage   88.98%   89.56%   +0.58%     
==========================================
  Files          11       11              
  Lines         890      930      +40     
==========================================
+ Hits          792      833      +41     
+ Misses         98       97       -1