Skip to content

Comments

add github action#64

Merged
charkour merged 5 commits intocharkour:mainfrom
BrunnerLivio:chore/ci
Apr 15, 2023
Merged

add github action#64
charkour merged 5 commits intocharkour:mainfrom
BrunnerLivio:chore/ci

Conversation

@BrunnerLivio
Copy link
Contributor

:)

@charkour
Copy link
Owner

Looks cool! What is junit?

@BrunnerLivio
Copy link
Contributor Author

BrunnerLivio commented Jan 10, 2023

@charkour In general junit is a report of your test cases written in XML which is supported by most test runners.

I added an action which will read that junit file (generated from vitest) and then should display a summary in the PR. It's totally optional though

My main motivation was simply to run the test & build during the Pipeline so you don't need to checkout PRs locally to verify whether the tests work

@charkour
Copy link
Owner

That's awesome! We can give it a go!

Never got around to adding the action back so this will be nice.

Are the results supposed to add a comment somewhere?

@BrunnerLivio
Copy link
Contributor Author

BrunnerLivio commented Jan 11, 2023

@charkour hmm seems like it doesn't have sufficient permissions -- maybe because I am on a fork.

It's up to you whether you wanna go after it -- it will be hard from my side to fix this since I am not the repo owner. You can also simply delete that step if it's not useful or too much effort to get it working. I believe the main merit here is that the build/test at least runs through!

EDIT: Ah found it here. I guess you'd need to create a Personal Access Token, and store it as a variable within this repositories actions variables.

This will be the result if it works:

@charkour
Copy link
Owner

Awesome, I've added a PAT to this repo. Looks like there might need to be two workflows. One for the test and another to publish the results. The secret is called JUNIT_PAT.

Is this something you'd like to work on @BrunnerLivio?

@BrunnerLivio BrunnerLivio force-pushed the chore/ci branch 10 times, most recently from 584e539 to b3cf885 Compare January 15, 2023 11:59
@BrunnerLivio
Copy link
Contributor Author

@charkour I was unfortunately unable to get it working with the report, but at least a more basic pipeline runs through nicely. I also added a matrix build for node v14,v16,18

@charkour
Copy link
Owner

Sounds good! I like the testing matrix.

The documentation for jUnit isn't super great. But for the commenting to work, it looks there two Workflows need to be run.

@charkour charkour self-requested a review January 15, 2023 17:27
Copy link
Owner

@charkour charkour left a comment

Choose a reason for hiding this comment

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

Would you be willing to add the junit.xml file to the .gitignore?

Then I think this is good to go

Copy link
Owner

@charkour charkour left a comment

Choose a reason for hiding this comment

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

oops I meant to request changes for this!

@BrunnerLivio
Copy link
Contributor Author

@charkour Oups, that file was meant to be deleted anyway. Not needed to be added to gitignore since it won't get generated anymore!

@charkour
Copy link
Owner

Thank you!

Copy link
Owner

@charkour charkour left a comment

Choose a reason for hiding this comment

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

Will be nice to run tests before merging. Thanks!

@charkour charkour merged commit e27c85f into charkour:main Apr 15, 2023
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