Comment 3 for bug 712913

Dear Minh,

I really appreciate the amount of work you have put into this and your other patches. However, with this test suite I have some concerns.

igraph_ring() is a pretty simple function. OK, this is because it calls igraph_lattice(), which can do a bit more than just generating ring and line graphs. Still, igraph_lattice() is less than 100 lines of code. Your test suite is about 800 lines. This seems to be out of proportion for me.

We need to maintain this code in the future, and even if you don't change the testing code too often, sometimes you need to, e.g. API changes are quite frequent in igraph, because a lot of the API is messy, or new functionality is added. In these cases we need to change the tests as well. And this requires effort and time. (In general, I'm not talking about igraph_ring() now.)

As for igraph_ring(), I would prefer some more concise tests, e.g. I don't see any need checking the girth of the graph, or even the number of edges. Just test the boundary cases, i.e. 0, 1, 2, 3, vertices, query the full edge list to see that it is all right and that's it.

Or maybe just store the edges of the correct graphs, and then see whether they are isomorphic to the ones create by igraph_ring(). Maybe this is the simplest, and it can be done with more readable code. I.e. something like:

igraph_real_t edges_undirected_circular_3[] = {0,1, 1,2, 2,0 };
igraph_real_t edges_undirected_line_3[] = {0,1, 1,2 };
...
igraph_ring(&g, 3, /*directed=*/ 0, /*mutual=*/ 0, /*circular=*/ 1);
if (! check_ring(&g, edges_undirected_circular)) return 1;
....

where check_ring() creates a graph from the given edge list and checks that it is isomorphic to the supplied graph.

What do you think?

Best,
Gabor