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

Try traget pull_request instead of pull_request_target #152

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Conversation

Bouni
Copy link
Owner

@Bouni Bouni commented Feb 5, 2024

This is an attempt to fix the pytest issue surfaced in #151

@Bouni
Copy link
Owner Author

Bouni commented Feb 5, 2024

@gerw My guess was wrong, this prints (HEAD detached at pull/152/merge) 1fcdeb4 Merge c97a155cc37e44e42515b7721cf4b8340826235d into 3d94a5903fe1e104531bd481e4277dfc11668f1a for git branch -v 😕

@kbabioch Do you have an idea why the test seem to run on the wrong files ?

Edit: I mean why the tests in #151 do seemingly not run against the files in the PR itself

Copy link

github-actions bot commented Feb 5, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
luxtronik
   __init__.py14311023%39–55, 62–67, 70, 74–79, 84–88, 101–104, 111, 115, 119, 123, 134, 137–140, 143–163, 166–181, 184–201, 204–219, 223–225, 229–230, 234–235
   __main__.py21210%3–49
   datatypes.py2751296%38, 43, 48, 58, 73–76, 81–84, 93
   discover.py433421%26–78
luxtronik/scripts
   dump_changes.py44440%5–93
   dump_luxtronik.py28280%5–64
TOTAL63724961% 

Tests Skipped Failures Errors Time
116 4 💤 0 ❌ 0 🔥 0.681s ⏱️

@Bouni
Copy link
Owner Author

Bouni commented Feb 5, 2024

Ok I think pull_request_target is indeed the problem, the tests for the trigger pull_request succeed!

@Bouni
Copy link
Owner Author

Bouni commented Feb 5, 2024

I would like to merge the pytest CI workflow with pull_request_target replaced with pull_request and see if that works for #151 as well.

At the moment this fails becuause the test file contains -7 in main and my branch but I don't want to fix this in here now so that we see if #151 succeds when rebased after this one merged.

Any thoughts?

@Bouni Bouni requested a review from gerw February 5, 2024 15:24
@gerw
Copy link
Collaborator

gerw commented Feb 5, 2024

LGTM. Thank you!

@gerw gerw merged commit e99926f into main Feb 5, 2024
6 of 10 checks passed
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.

2 participants