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

Migrate tests of matrix_of_constraints.jl #1457

Merged
merged 1 commit into from
Jul 23, 2021
Merged

Migrate tests of matrix_of_constraints.jl #1457

merged 1 commit into from
Jul 23, 2021

Conversation

odow
Copy link
Member

@odow odow commented Jul 14, 2021

Part of #1398

Interested to see what this does to the code coverage. The previous tests in this file were pretty haphazard, and it's not obvious which tests were testing what.

The individual components (ProductOfSets, MutableSparseMatrixCSC, etc.) are independently tested, so I don't see the need for wide-scale testing on lots of different types.

@odow odow added the Submodule: Utilities About the Utilities submodule label Jul 14, 2021
@odow odow requested a review from blegat July 14, 2021 02:43
@odow odow mentioned this pull request Jul 14, 2021
22 tasks
@odow
Copy link
Member Author

odow commented Jul 14, 2021

That's quite a large coverage drop, so I'll hold off merging until that is resolved. This is a good example that refactoring the tests is useful! The files should be independently tested. We shouldn't have a big drop in coverage for sparse_matrix.jl by changing these tests.

Of course, it makes my earlier statement incorrect at present:

The individual components (ProductOfSets, MutableSparseMatrixCSC, etc.) are independently tested, so I don't see the need for wide-scale testing on lots of different types.

😢

image

@odow odow closed this Jul 16, 2021
@odow odow reopened this Jul 16, 2021
@odow odow force-pushed the od/test-moc branch 3 times, most recently from b6b48f1 to ec6fece Compare July 21, 2021 22:35
@odow
Copy link
Member Author

odow commented Jul 22, 2021

This is now in a better state:
image
Coverage increases for this file. Large drop is due to DeprecatedTests. We can fix the other regressions in a different PR.

@odow odow merged commit 618293b into master Jul 23, 2021
@odow odow deleted the od/test-moc branch July 23, 2021 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Submodule: Utilities About the Utilities submodule
Development

Successfully merging this pull request may close these issues.

1 participant