GoVetBear - skip condition invalid

[sils]

From @AbdealiJK on February 9, 2016 3:20

I have ubuntu 14.04 and was trying to get all the tests in coala to work for me.
I noticed that GoVetBearTest kept failing and found out that in GoVet the skip condition is set to go.
This is not right because that isn't the correct thing to check. I had go installed but not go vet.

I had to install golang-go.tools to get the vet package in go.

Copied from original issue: coala/coala#1329

[sils]

From @AbdealiJK on February 9, 2016 5:43

The skip condition to be to check if go help vet returns 0 or not

[sils]

From @NalinG on February 9, 2016 10:11

Hi, I would like to work on this issue.
I checked the GoVetBear.py file but is just called the self.lint method.
Do we have to change the lint method or I am missing something.

Thank You

[sils]

From @AbdealiJK on February 9, 2016 10:30

The ship condition is in GoVetBearTest
The @SkipIf checks for go, not for go vet

-----Original Message-----
From: "Nalin Goel" notifications@github.com
Sent: ‎09-‎02-‎2016 15:41
To: "coala-analyzer/coala" coala@noreply.github.com
Cc: "AbdealiJK" abdealikothari@gmail.com
Subject: Re: [coala] GoVetBear - skip condition invalid (#1329)

Hi, I would like to work on this issue.
I checked the GoVetBear.py file but is just called the self.lint method.
Do we have to change the lint method or I am missing something.
Thank You
—
Reply to this email directly or view it on GitHub.

[sils]

@NalinG please work only on one newcomer issue, we don't have a lot of them because they take a lot of work to create.

[sils]

From @NalinG on February 9, 2016 18:41

Sorry for the trouble.
I will take care in the future.

[sils]

no problem

[manu-chroma]

Hey @sils1297, I'd like to work on this issue! :)

[manu-chroma]

@AbdealiJK using which('go help vet ') returns None even though I have go vet installed.

I can use os.system('go help vet') in place for checking installation. Thoughts ?

[AbdealiLoKo]

'which' doesn't work like that. It only accepts 1 arg. What were you expecting 'which go vet bear' to give???

Instead of os.system() we use sub process.check_call or subprocess.check_output. Look at other bears and how it's implemented in them.

[manu-chroma]

I realised that after posting the comment. Sure, I'll do that!
Thanks.

[jayvdb]

Please see the new GoRequirement . Each bear should not do their own package management checks - they will be buggy.

[jayvdb]

(Go vet is a bit special, but its specialness should be in GoRequirement)

[manu-chroma]

@jayvdb I'm not sure if you're pointing to any specific part of the documentation.
Can you give a link for where I can see the GoRequirement..

[AbdealiLoKo]

@jayvdb but we need to ensure it works for the test skipping right? Or has that been offloaded to GoRequirements?
CC @Adrianzatreanu

[jayvdb]

See coala/coala#2752 , which fixed GoRequirement.is_installed for the general case. Special tests might be needed for go vet.

Test skipping should be done using GoRequirement.is_installed, and can probably be done dynamically in LocalBearTestHelper as the requirement is declared in the bear.

[manu-chroma]

@AbdealiJK @jayvdb So the condition for checking go vet should be implemented in GoRequirement.is_installed or should I put it in GoVetBear.py inplace of which(go) implementation ?

[sils]

Ideally it'd be in GoRequirement.is_installed done properly and then the Bear class should automatically use those. CC @Makman2 and @Udayan12167 as this affects the Bear and LintBase classes

[manu-chroma]

@sils Since @Makman2 is working on this issue, should I pick a different newcomer issue ?

[sils]

@manu-chroma sure, if you like. Not sure what @Makman2 is planning