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

Fix test #151

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Fix test #151

merged 1 commit into from
Feb 7, 2024

Conversation

gerw
Copy link
Collaborator

@gerw gerw commented Feb 5, 2024

Fixes #150

@gerw
Copy link
Collaborator Author

gerw commented Feb 5, 2024

Am I being crazy? Why does pytest fails exactly at the line which I have changed? Does it test the previous version of the code?

@Bouni
Copy link
Owner

Bouni commented Feb 5, 2024

🤔 Can you try to rebase onto main? Maybe something went wrong there !?

@gerw
Copy link
Collaborator Author

gerw commented Feb 5, 2024

I think there is nothing to rebase here. This PR is directly one commit below main, isn't it?

@Bouni
Copy link
Owner

Bouni commented Feb 5, 2024

I've just checked, your branch is up to date with the upstream main branch except the single line change.

But I think I found the issue, the CI job does a check out of the main branch, not your PR branch fix150:

https://github.com/Bouni/python-luxtronik/actions/runs/7785513708/job/21228279923?pr=151#step:2:51

Fetching the repository
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +3d94a5903fe1e104531bd481e4277dfc11668f1a:refs/remotes/origin/main

@gerw
Copy link
Collaborator Author

gerw commented Feb 5, 2024

Now, the publishing of the code coverage seems to fail. I think we already had this issue at some time in the past?

@Bouni
Copy link
Owner

Bouni commented Feb 5, 2024

I'll look into it tomorrow

@Bouni
Copy link
Owner

Bouni commented Feb 5, 2024

Guess I found a solution:

MishaKav/pytest-coverage-comment#68 (comment)

Will add the permissions tomorrow

@Bouni
Copy link
Owner

Bouni commented Feb 6, 2024

Ok, Chat GPT sucks as it proposed wrong inputs for the coverage action 🤦🏽‍♂️
I simply don't understand why it works if I do a PR but not for you ...

Maybe because I'm the owner of the repo and you just a contributer!?

I'll try to find another solution ...

@gerw
Copy link
Collaborator Author

gerw commented Feb 6, 2024

No, I think that the problem is that your PRs come directly from this repo, but my PR come from a different repo. In principle, everybody could file a PR with (potentially malicious) code and this should not be executed with the permissions of this repo.

@gerw
Copy link
Collaborator Author

gerw commented Feb 6, 2024

What happens if you change repo-token to github-token?

@Bouni
Copy link
Owner

Bouni commented Feb 6, 2024

I don't know, lets try.

@Bouni
Copy link
Owner

Bouni commented Feb 6, 2024

I just changed the actions settings in the hope that it fixes the problem, can you do git commit --allow-empty -m "Trigger-CI" to see if that works?

@gerw
Copy link
Collaborator Author

gerw commented Feb 6, 2024

Now, the workflows seem to fail already at startup...

@Bouni
Copy link
Owner

Bouni commented Feb 6, 2024

Ok then I'll put the settings back to original and continue to search for a solution

@Bouni
Copy link
Owner

Bouni commented Feb 6, 2024

I guess I found the answer in this blog post

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

I'll try to create a two staged coverage ci workflow tomorrow

@Bouni
Copy link
Owner

Bouni commented Feb 7, 2024

@gerw Now everything runs but MishaKav/pytest-coverage-comment@main gives this error:

Warning: This action supports comments only on `pull_request`, `pull_request_target`, `push` and `workflow_dispatch`  events. `workflow_run` events are not supported.

So still no coverage report for forked repo PRs but at least the pytest CI workflow works.

I'll see if I can fix this somehow. Anyway, now we can continue with the real work on python-luxtronik 🥳

@Bouni Bouni merged commit 7145818 into Bouni:main Feb 7, 2024
4 checks passed
@gerw gerw deleted the fix150 branch February 20, 2024 21:13
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.

Datatypes test fails
3 participants