✨ Add more info to logs

[laurentsimon]

Add more info to logs to help troubleshooting problems.
There's been a new issue on a problem that we've seen before #154

cc @naveensrinivasan let's add this to our list of unit tests once we've figured the problem.

Add more info to logs

closes #156

[codecov]

Codecov Report

Merging #155 (731c248) into main (bc84c34) will not change coverage.
The diff coverage is n/a.

❗ Current head 731c248 differs from pull request most recent head 36a22b6. Consider uploading reports for the commit 36a22b6 to get more accurate results

@@           Coverage Diff           @@
##             main     #155   +/-   ##
=======================================
  Coverage   74.80%   74.80%           
=======================================
  Files           2        2           
  Lines         127      127           
=======================================
  Hits           95       95           
  Misses         25       25           
  Partials        7        7           

[naveensrinivasan]

LGTM! Can we please also include this in the go code?

[laurentsimon]

LGTM! Can we please also include this in the go code?

IIUC the go code in out of sync already. Example: it's reading from the GITHUB_EVENT_PATH file https://github.com/ossf/scorecard-action/blob/main/options/options.go#L229 instead of using https://github.com/ossf/scorecard-action/blob/main/github/github.go#L115

I'll create an issue for sync'ing the code.

[azeemshaikh38]

LGTM! Can we please also include this in the go code?

+1 to this. It's easier to just do this as we go along. Even better, we can consider only adding updates to the Golang code instead of bash.

[laurentsimon]

LGTM! Can we please also include this in the go code?

+1 to this. It's easier to just do this as we go along. Even better, we can consider only adding updates to the Golang code instead of bash.

I'll try that. I cannot add code only to go code though, because I have no idea if it does the same as entrypoint.sh. At least it should be used by our own repos to stage a release.

[laurentsimon]

LGTM! Can we please also include this in the go code?

+1 to this. It's easier to just do this as we go along. Even better, we can consider only adding updates to the Golang code instead of bash.

I'll try that. I cannot add code only to go code though, because I have no idea if it does the same as entrypoint.sh. At least it should be used by our own repos to stage a release.

done, PTAL

[laurentsimon]

please also advise how you'd like me to test. It used to be a file with the content, now it's an live API call to GitHub. I think I'll have to use the mock library; but tell me if you had something else in mind.
@naveensrinivasan could you point me to a good example in scorecard you'd like me to follow?

[azeemshaikh38]

please also advise how you'd like me to test. It used to be a file with the content, now it's an live API call to GitHub. I think I'll have to use the mock library; but tell me if you had something else in mind. @naveensrinivasan could you point me to a good example in scorecard you'd like me to follow?

Do you mean test Golang code? @rohankh532 is working on adding e2e tests similar to how we test bash script today so can skip that for now. And the bash changes should get tested using the e2e we have already.

[laurentsimon]

please also advise how you'd like me to test. It used to be a file with the content, now it's an live API call to GitHub. I think I'll have to use the mock library; but tell me if you had something else in mind. @naveensrinivasan could you point me to a good example in scorecard you'd like me to follow?

Do you mean test Golang code? @rohankh532 is working on adding e2e tests similar to how we test bash script today so can skip that for now. And the bash changes should get tested using the e2e we have already.

The code change in go now queries a GitHub API to retrieve repo information (instead of using a file content GITHUB_EVENT_PATH), so I need to mock the API if I want to have unit tests for it. The tests are failing right now, and I suspect it's because of that. I'm more than happy to leave this to @rohankh532... but that means go changes cannot be included in this PR... Wdut?

[naveensrinivasan]

@laurentsimon Is this still valid?

[laurentsimon]

Let's close.