loadsmart / danger-android_lint

A Danger plugin for Android Lint
https://developer.android.com/studio/write/lint.html
MIT License
55 stars 24 forks source link

[WIP] Add `report_file` parameter to Android_lint plugin #2

Closed churowa closed 7 years ago

churowa commented 7 years ago

This adds a new option to the android lint plugin, allowing the user to set their report file location in the Dangerfile, using something like:

android_lint.report_file = "/app/build/output/lint/lint.xml"

The default report file location is at: 'app/build/reports/lint/lint-result.xml'

churowa commented 7 years ago

I think I might need to update some tests. I'm not quite sure where to add them though.

barbosa commented 7 years ago

Hey @churowa

Do you want me to merge this and add the tests later or do you want to give it a try?

churowa commented 7 years ago

I'd definitely like to try, if that's ok. It's a good learning experience for me. Sorry it's taking so long! Hard to find time.

barbosa commented 7 years ago

No worries @churowa !

Just wanted to make sure you want to do it! No rush 😄

churowa commented 7 years ago

Actually @barbosa I could use a hand. I just pushed a couple of tests. One of them works ok, but the other is failing. Honestly, I'm not sure what the following means:

#<File (class)> received :exists? with unexpected arguments
         expected: ("app/build/reports/lint/lint-result.xml")
              got: ("some/other/location/lint-result.xml")
        Please stub a default value first if message might be received with other args as well. 

It looks like the plugin isn't taking the value for report_file that I'm trying to set, and is trying to use the default value instead. Is that correct?

barbosa commented 7 years ago

Oh, I see.

That's because when you do:

allow(File).to receive(:exists?).with(@android_lint.report_file).and_return(true)

at this point of the execution, @android_lint.report_file is not set yet, so it returns its default value. The test will store this default value as the value it expects when running @android_lint.lint, so it fails.

To fix that, you should set the value before the stub, like:

@android_lint.report_file = 'some/other/location/lint-result.xml'
allow(File).to receive(:exists?).with(@android_lint.report_file).and_return(false)

That might fix the test.

Cheers! 🍻

churowa commented 7 years ago

Ok! All tests passing now @barbosa. :)

Thanks for the pointer. It all makes more sense now to me.

barbosa commented 7 years ago

That looks great!

Thanks for your contribution, @churowa 🎉

barbosa commented 7 years ago

Just released version 0.0.3: https://rubygems.org/gems/danger-android_lint/versions/0.0.3 💎