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

Add test case for FLOWS/FLORES with NNC and BCCON #1134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

totto82
Copy link
Member

@totto82 totto82 commented Feb 9, 2024

Motivation.
OPM/opm-models#874

@totto82
Copy link
Member Author

totto82 commented Feb 13, 2024

The fix is merged. I really want to add this to regression to avoid further issues. Review?

@bska
Copy link
Member

bska commented Feb 13, 2024

I really want to add this to regression to avoid further issues.

Okay, but maybe you can start adding some real unit tests as well? Regression tests are good as far as they go, but it's a lot easier to catch problems with tests that don't require building and running the full simulator.

@totto82
Copy link
Member Author

totto82 commented Feb 13, 2024

Okay, but maybe you can start adding some real unit tests as well? Regression tests are good as far as they go, but it's a lot easier to catch problems with tests that don't require building and running the full simulator.

I dont think it would be easy to make a simple unit test for this without including the full assembly code and the output code, for that we need to include the full simulator anyway. Yes, unit tests are good to have, but regression tests are better than nothing.

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