slack-ruby / slack-ruby-client

A Ruby and command-line client for the Slack Web, Real Time Messaging and Event APIs.
MIT License
1.19k stars 215 forks source link

CI failing on deprecation warnings #409

Open dblock opened 2 years ago

dblock commented 2 years ago

Need to relax the test a bit to allow for deprecations, and fix the deprecations.

https://github.com/slack-ruby/slack-ruby-client/runs/6746428976?check_suite_focus=true

  1) Slack globals requires --slack-api-token
     Failure/Error:
       expect(err).to(
         start_with(
           'error: parse error: Set Slack API token via --slack-api-token or SLACK_API_TOKEN.'
         )
       )

       expected "Top level ::CompositeIO is deprecated, require 'multipart/post' and use `Multipart::Post::CompositeR...lack [global options] auth test \n\n\n\nDESCRIPTION\n    Checks authentication & identity. \n\n\n\n" to start with "error: parse error: Set Slack API token via --slack-api-token or SLACK_API_TOKEN."
     # ./spec/slack/slack_spec.rb:35:in `block (3 levels) in <top (required)>'
     # /Users/dblock/.rvm/gems/ruby-2.7.5/gems/webmock-3.14.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
schinery commented 2 years ago

This is cause by the multipart-post gem being updated to 2.2.0, which is used by the faraday-mulitpart gem. There is this issue to udate the gem and remove the dependency warning.

dblock commented 2 years ago

Thanks for hunting it down @schinery. I still want a PR here that ignores those warnings and where tests pass.

schinery commented 2 years ago

Thanks for hunting it down @schinery. I still want a PR here that ignores those warnings and where tests pass.

Ah, sorry I see what you mean now.

I've taken a quick look at trying to get the tests to pass, 2 of them are straight forward in making the assertion looser but the 2 fails that are calling json = Slack::Messages::Message.new(JSON[#{command}]) are a bit tricker as the warnings are returned in with the JSON string by the command call.

The only way I've got these tests running so far is to run them with RUBYOPT=W0 to suppress the warnings completely when they are run on CI (draft PR here), so it is a question on whether you think this is acceptable? Not really sure of another option though...

armilam commented 2 years ago

faraday-multipart has been updated: https://github.com/lostisland/faraday-multipart/pull/6

We might be able to update it again here to get rid of the warnings?

dblock commented 2 years ago

We're back to green in #411, but the issue here is still valid: there are tests that fail with a false positive if some gem prints out deprecation warnings. It's NBD of course since it only affects tests, and only rarely.