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

Repair unittest and test coverage pipeline #37

Merged
merged 14 commits into from
Jul 11, 2023
Merged

Repair unittest and test coverage pipeline #37

merged 14 commits into from
Jul 11, 2023

Conversation

Bronzila
Copy link
Collaborator

@Bronzila Bronzila commented Jul 6, 2023

This PR adds coverage results to the unittests. An appropriate badge both for passing all tests and the test coverage are added to the README. Moreover the toy test file is deleted and a new file testing the budget exhaustion has been added.

@Bronzila Bronzila requested a review from Neeratyoy July 6, 2023 12:00
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you also, write brief docstrings describing each unit test?
This would then become the template or guide for adding more unit tests.

@Neeratyoy
Copy link
Collaborator

Would be nice if all tests pass on this branch, before merging to dev.

Alternatively, you could use the PR to only merge the functionalities that are working and ✅.

@Bronzila
Copy link
Collaborator Author

Would be nice if all tests pass on this branch, before merging to dev.

Alternatively, you could use the PR to only merge the functionalities that are working and ✅.

Currently all Tests pass, the only non-passing actions are pre-commit and docs. I was planning to get these two running on separate branches so that the PRs don't get too large. Would you prefer that I fix them all on one branch?

@Neeratyoy
Copy link
Collaborator

I was planning to get these two running on separate branches so that the PRs don't get too large. Would you prefer that I fix them all on one branch?

If it is not too troublesome, we could merge only the unit testing bit and leave these 2 out.
Alternatively, we merge this, and you create a new branch from this merged dev, where a new PR comes in to fix the 2 failing tests for docs and styling.
Whatever you decide as the Occam's razor, especially if fixing the 2 failures is not going to be a time sink.

@Bronzila
Copy link
Collaborator Author

If it is not too troublesome, we could merge only the unit testing bit and leave these 2 out. Alternatively, we merge this, and you create a new branch from this merged dev, where a new PR comes in to fix the 2 failing tests for docs and styling. Whatever you decide as the Occam's razor, especially if fixing the 2 failures is not going to be a time sink.

I would prefer merging this and then creating a new branch with a new PR to fix the docs and styling actions. I don't think it will take too much time to fix these workflows and I think the time is well invested, since we will then have a solid pipeline for clean further developments.

@Bronzila Bronzila merged commit d9da120 into development Jul 11, 2023
12 of 14 checks passed
@Bronzila Bronzila deleted the unittests branch July 18, 2023 13:44
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.

None yet

2 participants