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

add unit tests coverage #1983

Closed
wants to merge 4 commits into from
Closed

add unit tests coverage #1983

wants to merge 4 commits into from

Conversation

wkloucek
Copy link
Contributor

@wkloucek wkloucek commented Aug 11, 2021

Adds coverage on unit tests to Codacy. Equals the setup at oCIS, where you can see the coverage on the dashboard. But I don't know why you cant see it at the files level though...

Manual check:

  • make test
  • go tool cover -func coverage.out
  • returns total: (statements) 37.8%

@update-docs
Copy link

update-docs bot commented Aug 11, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@wkloucek
Copy link
Contributor Author

@labkode this needs a codacy_token token secret to work. See also http://plugins.drone.io/drone-plugins/drone-codacy/

@wkloucek wkloucek marked this pull request as ready for review August 11, 2021 09:04
@wkloucek wkloucek requested a review from labkode as a code owner August 11, 2021 09:04
@wkloucek wkloucek requested a review from micbar August 11, 2021 09:04
@ishank011
Copy link
Contributor

@wkloucek I added the token and it seems to work now, but should we restrict sending the report only on merges?

@wkloucek
Copy link
Contributor Author

@wkloucek I added the token and it seems to work now, but should we restrict sending the report only on merges?

should also be on master, will have a look tomorrow

},
"trigger":{
"branch":[
"master"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ishank011 master should now also run the coverage report

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change event to push?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want you also can edit it directly. I must confess that I am not really familiar with the include / exclude logic of Drone since we are using the reference format for achieving the same "trigger": { "ref": [ "refs/heads/master", "refs/pull/**", ], },

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why we need push because PR and master should be the only cases!?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"trigger": { "ref": [ "refs/heads/master", "refs/pull/**", ], }, is working fine in all owncloud repos

@butonic
Copy link
Contributor

butonic commented Aug 16, 2021

@labkode I'm not sure this is ready. @wkloucek @ishank011 and @micbar are on vacation for 2 weeks at least. Would be a shame to let this bitrot.

@labkode
Copy link
Member

labkode commented Aug 16, 2021

Hi @butonic I think we only need to address the last comment from @micbar

@C0rby C0rby mentioned this pull request Aug 18, 2021
@C0rby
Copy link
Contributor

C0rby commented Aug 20, 2021

Superseded by #2004

@C0rby C0rby closed this Aug 20, 2021
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.

6 participants