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

Tweak Github Actions support to match official app #21

Closed
2 tasks done
alerque opened this issue Sep 23, 2019 · 11 comments · Fixed by #23
Closed
2 tasks done

Tweak Github Actions support to match official app #21

alerque opened this issue Sep 23, 2019 · 11 comments · Fixed by #23
Assignees

Comments

@alerque
Copy link
Contributor

alerque commented Sep 23, 2019

The official Coveralals app for Github Actions (not useful for Lua, but great otherwise) uses an internal parameter github-token as the identifier instead of having to pass a custom secret token.

See related upstream issue here.

We should update the support here to do the same thing.

  • Use github as the action name
  • Drop the custom token handling and pass github-token
@alerque
Copy link
Contributor Author

alerque commented Sep 23, 2019

(If you can feel free to assign this issue to me so I don't forget about it.)

@moteus
Copy link
Owner

moteus commented Sep 23, 2019

Do I meiss some think but github-token pass just as regular repo_token and service name is just github
Token if i understend correctly have to be passed via command line, service name can be also passed via command line arguments.
Do you whant to calculate these parameters?

@LPGhatguy
Copy link

My team recently switched from Travis-CI to GitHub Actions and ran into this discrepancy! You need to pass github as the service name or luacov-coveralls won't be able to handle the GitHub token when passed as the repo token.

We run this right now:

- name: Report to Coveralls
  run: luacov-coveralls --repo-token $REPO_TOKEN --service-name github
  env:
    REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}

But it would be nice to instead be able to just write:

- name: Report to Coveralls
  run: luacov-coveralls --repo-token $REPO_TOKEN
  env:
    REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
```

I think the fix ends up being very simple. Instead of using `actions` as the service name when GitHub Actions is detected, use `github` instead?

@alerque
Copy link
Contributor Author

alerque commented Jan 15, 2020

@LPGhatguy I just submitted a PR (#23) to fix the service name, and also I think if the env variable is set right in the first pace you can skip the repo token arg too:

- name: Report to Coveralls
  run: luacov-coveralls
  env:
    COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Would you be able to test this by chance?

@LPGhatguy
Copy link

LPGhatguy commented Jan 15, 2020

I took a stab at this in the PR that GitHub marked as related to this issue. It looks like the configuration I tried didn't work out. I can look into this more detail if that isn't enough information!

EDIT: I don't think I installed the version of luacov-coveralls from your branch correctly, oops. LuaRocks is a little funky to try to navigate.

@alerque
Copy link
Contributor Author

alerque commented Jan 16, 2020

@LPGhatguy Thanks for the help testing this in a real world scenario! That means a lot.

I think what's happening is that even though you check out the right source, the rockspec you then use goes back and fetches the old released source. I'll look and see if I can find a way to do it from the local tree.

@alerque
Copy link
Contributor Author

alerque commented Jan 16, 2020

@moteus I have confirmed via this test run that these proposed changes do work as expected, I think this can be merged now.

@moteus
Copy link
Owner

moteus commented Jan 17, 2020

In your job you got Upload error: Couldn't find a repository matching this job. is it expected? also to be sure that uses correct version of the luaco-coveralls it is possible to add -v flag.

@alerque
Copy link
Contributor Author

alerque commented Jan 17, 2020

@moteus I think what you are seeing is a false alarm. Github is not showing test results for the commit I actually wanted to show off. I had pushed a temporary commit with the special rockspec to get this branch (and also ran with -v), and it all worked fine. Then I forced push that commit off and went back to a cleaned up version in anticipation of this fix being released. The result is the error you see now. I poked around and can't even find the logs for the now non-existent commit. My link above does not go to what I actually linked at the time.

@Ismoh
Copy link

Ismoh commented Feb 1, 2022

@LPGhatguy I just submitted a PR (#23) to fix the service name, and also I think if the env variable is set right in the first pace you can skip the repo token arg too:

- name: Report to Coveralls
  run: luacov-coveralls
  env:
    COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Would you be able to test this by chance?

I was struggling for days to get this running.
The important change is to use the github token instead the coveralls token.
This isnt mentioned anywhere in the docs or in the lua-example repo.

Can anyone add this for the ppl, who might struggle with this in future?

- name: Generate coverage report with luacov-coveralls
      env:
        #github-token: ${{ secrets.github_token }}
        #COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
        COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        COVERALLS_DEBUG: true
      run: |
        ${{ github.workspace }}/luajit/bin/luacov-coveralls -v

@Ismoh
Copy link

Ismoh commented Feb 2, 2022

In addition to the comment above, this is an awesome explanation and could be use as a reference:
lemurheavy/coveralls-public#1613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants