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

Xcode 9 support without extra puts #338

Closed
wants to merge 10 commits into from
Closed

Conversation

FDREAL
Copy link

@FDREAL FDREAL commented Sep 14, 2017

Some time ago @ivanbruel opened #321 to support Xcode 9. In the last commit, he added an extra puts that interferes with the tests.

I am removing the puts hoping a new Slather release can be made soon.

@tzm41
Copy link

tzm41 commented Sep 14, 2017

👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 95.658% when pulling 2092968 on FDREAL:master into f478561 on SlatherOrg:master.

@ksuther
Copy link
Contributor

ksuther commented Sep 14, 2017

Thanks, I'll take a look at this soon.

@ksuther
Copy link
Contributor

ksuther commented Sep 14, 2017

Are you seeing the tests pass with Xcode 9? I'm still seeing failures. I'm looking into why, but I'm not sure if anyone else is seeing the same thing (or if you're still running the unit tests with Xcode 8 and only testing the slather executable with Xcode 9).

@FDREAL
Copy link
Author

FDREAL commented Sep 15, 2017

I only have Xcode GM installed. It may depend on different configurations if you're seeing errors. I'll try one more time with a different setup.

If you see anything, I'll be glad to help solve that.

@ksuther
Copy link
Contributor

ksuther commented Sep 15, 2017

I've fixed some of the failing tests, they're failing because the tests are expecting the Xcode 8 layout for Coverage.profdata. I have some of the tests fixed for Xcode 9 and I'll make a release once I have a chance to test out some more changes I made against Xcode 9.

@FDREAL
Copy link
Author

FDREAL commented Sep 15, 2017

That's great. Do you need help with something?

@FDREAL
Copy link
Author

FDREAL commented Sep 15, 2017

I've just re-read your answer. Is weird it doesn't fail on my end if it is a problem with the new layout.
I'll be investigating tomorrow.

@ksuther
Copy link
Contributor

ksuther commented Sep 15, 2017

I am seeing these failures:

rspec ./spec/slather/project_spec.rb:147 # Slather::Project#binary_file should find the product path provided a scheme
rspec ./spec/slather/project_spec.rb:155 # Slather::Project#binary_file should find the product path provided a workspace and scheme
rspec ./spec/slather/project_spec.rb:164 # Slather::Project#binary_file should find the product path for a scheme with no buildable products
rspec ./spec/slather/project_spec.rb:172 # Slather::Project#binary_file should find multiple unique paths for a scheme with serveral buildable/testable products
rspec ./spec/slather/project_spec.rb:203 # Slather::Project#binary_file should configure the binary_basename from yml
rspec ./spec/slather/project_spec.rb:509 # Slather::Project#verbose_mode should print out environment info when in verbose_mode
rspec ./spec/slather/project_spec.rb:523 # Slather::Project#verbose_mode should print error when no binaries found

This is when running tests with bundle exec rake

The changes I've made are here: https://github.com/ksuther/slather/tree/xcode9

@FDREAL
Copy link
Author

FDREAL commented Sep 15, 2017

@ksuther I ran the tests again and I was able to see 4 failures with your latest code:

rspec ./spec/slather/project_spec.rb:509 # Slather::Project#verbose_mode should print out environment info when in verbose_mode
rspec ./spec/slather/project_spec.rb:523 # Slather::Project#verbose_mode should print error when no binaries found
rspec ./spec/slather/project_spec.rb:602 # Slather::Project#find_binary_files load configuration from xcsheme search binary from 'Products/Debug*'
rspec ./spec/slather/project_spec.rb:609 # Slather::Project#find_binary_files load configuration from option search binary from 'Products/Release*

@ksuther ksuther mentioned this pull request Sep 17, 2017
@ksuther
Copy link
Contributor

ksuther commented Sep 19, 2017

Merged #339 based on these changes. Thanks for your contribution!

@ksuther ksuther closed this Sep 19, 2017
@FDREAL
Copy link
Author

FDREAL commented Sep 22, 2017

@ksuther Thank you and sorry I was not able to fix those tests, I ran into production issues.

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.

5 participants