-
Notifications
You must be signed in to change notification settings - Fork 1
Adding tests v1 #50
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
Adding tests v1 #50
Conversation
Putting the review on temporary hold, as there seems to be some path issue. |
I have updated the test setup configuration by enabling tests on all 3 platforms and have also fixed the missing path issue in the test setup. I think I encountered 2 Linux specific test failures, so I have filtered those out. I'll qualify them manually once and create new issues as needed. Please feel free to review the changes now. |
It looks like the upload code coverage step is failing saying that it requires a token. I'm not sure if you've already added one and it just wouldn't work for the testing branch or if you need to generate one and add it to the project. I'd also be curious as to what this setup looks like once code coverage is getting uploaded, given that all 3 platforms will upload code coverage. It might be worth separating the jobs out so code coverage only gets uploaded once if the current setup makes it harder to track changed coverage. Otherwise this looks good to me! |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Pulling from main
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved for now.
Will take an offline look in RB.
Added initial set of tests.