Add tests for layer.cpp by eivindjahren · Pull Request #1119 · equinor/resdata

eivindjahren · 2026-02-12T12:54:34Z

No description provided.

Copilot

Pull request overview

This PR adds comprehensive test coverage for the layer.cpp module using the Catch2 testing framework. The tests validate layer operations including cell value management, barriers, edge tracing, and grid integration.

Changes:

Added test_layer.cpp with 663 lines of test cases covering layer functionality
Updated CMakeLists.txt to include the new test file in the build configuration and add private-include directory access

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
lib/tests/test_layer.cpp	New comprehensive test suite for layer.cpp functionality including cell operations, barriers, edge tracing, and grid interactions
lib/CMakeLists.txt	Added test_layer.cpp to test suite build and included private headers directory

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T14:47:36Z

lib/tests/test_layer.cpp

+                int_vector_free(j_list);
+            }
+
+            THEN("The count of cells equal to 7 is thee") {


Corrected spelling of 'thee' to 'three'.

Suggested change

THEN("The count of cells equal to 7 is thee") {

THEN("The count of cells equal to 7 is three") {

Copilot · 2026-02-12T14:47:37Z

lib/tests/test_layer.cpp

+                }
+            }
+
+            AND_GIVEN("A 3x3x block at the far corner") {


Corrected 'A 3x3x block' to 'A 3x3 block' (removed extra 'x').

Suggested change

AND_GIVEN("A 3x3x block at the far corner") {

AND_GIVEN("A 3x3 block at the far corner") {

Copilot · 2026-02-12T14:47:37Z

lib/tests/test_layer.cpp

+                    }
+                }
+
+                THEN("Traceing from a starting point not on edge traces the "


Corrected spelling of 'Traceing' to 'Tracing'.

Suggested change

THEN("Traceing from a starting point not on edge traces the "

THEN("Tracing from a starting point not on edge traces the "

eivindjahren · 2026-02-13T12:08:02Z

Superseded by #1120

ajaust

I only have minor things and the pointers you have already fixed. You have to see if you want to change anything based on my comments. 🙂

ajaust · 2026-02-13T12:03:42Z

lib/tests/test_layer.cpp

+            }
+        }
+        AND_GIVEN("A layer of the same size") {
+            layer_type *dst = layer_alloc(nx, ny);


Candidate for unique_ptr.

ajaust · 2026-02-13T12:03:52Z

lib/tests/test_layer.cpp

+                }
+            }
+
+            layer_free(dst);


Can be deleted if we use a smart pointer.

ajaust · 2026-02-13T12:06:53Z

lib/tests/test_layer.cpp

+    GIVEN("A layer") {
+        int nx = 10;
+        int ny = 8;
+        layer_type *layer = layer_alloc(nx, ny);


I am not sure if this is freed if any of the tests fail before reaching the free.

I think we could wrap it in a smart like pointer like this.

Suggested change

layer_type *layer = layer_alloc(nx, ny);

std::unique_ptr<layer_type, decltype(&layer_free)> layer(layer_alloc(nx, ny), layer_free);

I am not sure about the decltype, but something like this should work.

ajaust · 2026-02-13T12:08:57Z

lib/tests/test_layer.cpp

+                int_vector_type *i_list = int_vector_alloc(0, 0);
+                int_vector_type *j_list = int_vector_alloc(0, 0);


Potential candidates for unique_ptr.

ajaust · 2026-02-13T12:09:09Z

lib/tests/test_layer.cpp

+                int_vector_free(i_list);
+                int_vector_free(j_list);


Can be removed if we use smart pointers.

ajaust · 2026-02-13T12:25:57Z

lib/tests/test_layer.cpp

+                        REQUIRE(layer_iget_cell_value(layer, i, j) == 7);
+                    }
+                }
+                REQUIRE(layer_get_cell_sum(layer) == 7 * 10 * 8);


Consider replacing the magic numbers 10 and 8 by nx and ny since they have names within the test.

Fixed this, should be in PR #1120

ajaust · 2026-02-13T12:29:41Z

lib/tests/test_layer.cpp

+        AND_GIVEN("A cell is set to some value") {
+            layer_iset_cell_value(layer, 2, 2, 10);
+
+            THEN("RIGHT and BOTTOM have that value for, negated for the "


I don't if I fully understand this THEN clause. Should it read "RIGHT and BOTTOM have that value, negated for the others"?

Does this THEN clause contain two strings on purpose? "RIGHT and BOTTOM have that value for, negated for the " and "others".

This was two strings because of formatting, I split it into two groups to be clear.

ajaust · 2026-02-13T12:35:04Z

lib/tests/test_layer.cpp

+                        for (int i = 0; i < 5; i++) {
+                            for (int j = 0; j < 5; j++) {


Are all cells connected or only the cells [0,5] in both directions?

ajaust · 2026-02-13T12:38:21Z

lib/tests/test_layer.cpp

+                THEN("Barrier is present at expected locations") {
+                    REQUIRE(layer_iget_left_barrier(layer, 2, 1) == true);
+                    REQUIRE(layer_iget_left_barrier(layer, 2, 2) == true);
+                    REQUIRE(layer_iget_left_barrier(layer, 3, 1) == false);


Could we use REQUIRE_FALSE here as in the tests above and below? I guess we could also remove the == true as this is not done in other tests.

Fixed everywhere.

ajaust · 2026-02-13T12:50:03Z

lib/tests/test_layer.cpp

+                    REQUIRE(corner_list ==
+                            std::vector<int_point2d_type>{{2, 2},
+                                                          {3, 2},
+                                                          {4, 2},
+                                                          {5, 2},
+                                                          {5, 3},
+                                                          {5, 4},
+                                                          {5, 5},
+                                                          {4, 5},
+                                                          {3, 5},
+                                                          {2, 5},
+                                                          {2, 4},
+                                                          {2, 3}});


I guess these values make sense. I did not look into the logic for defining the corners, but I find it somewhat confusing. We have a 3x3 block, but the list of corners is containing something like 2x2 block.

5 # # # # 4 # # j 3 # # 2 # # # # 1 1 2 3 4 5 i

I added your diagram to the test.

eivindjahren self-assigned this Feb 12, 2026

eivindjahren added this to SCOUT Feb 12, 2026

eivindjahren moved this to Ready for Review in SCOUT Feb 12, 2026

eivindjahren force-pushed the fix_layer branch from ca79697 to 47b1d51 Compare February 12, 2026 14:46

eivindjahren requested a review from Copilot February 12, 2026 14:46

Copilot AI reviewed Feb 12, 2026

View reviewed changes

Add tests for layer.cpp

c476b43

eivindjahren force-pushed the fix_layer branch from 47b1d51 to c476b43 Compare February 13, 2026 07:59

eivindjahren closed this Feb 13, 2026

github-project-automation bot moved this from Ready for Review to Done in SCOUT Feb 13, 2026

ajaust approved these changes Feb 13, 2026

View reviewed changes

github-project-automation bot moved this from Done to Reviewed in SCOUT Feb 13, 2026

	THEN("The count of cells equal to 7 is thee") {
	THEN("The count of cells equal to 7 is three") {

	AND_GIVEN("A 3x3x block at the far corner") {
	AND_GIVEN("A 3x3 block at the far corner") {

	THEN("Traceing from a starting point not on edge traces the "
	THEN("Tracing from a starting point not on edge traces the "

	layer_type *layer = layer_alloc(nx, ny);
	std::unique_ptr<layer_type, decltype(&layer_free)> layer(layer_alloc(nx, ny), layer_free);

		int_vector_type *i_list = int_vector_alloc(0, 0);
		int_vector_type *j_list = int_vector_alloc(0, 0);

Conversation

eivindjahren commented Feb 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

eivindjahren commented Feb 13, 2026

Uh oh!

ajaust left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants