Always close files (ReadDigraphs, WriteDigraphs) (#757)

* fix issues for read, add tests

* add tests

* close files with WriteDigraphs

* fix ci

* a little tidier

* implement max's suggestion

* fix tests

* rm debugging lines

Author: pramothragavan

gap/io.gi

@@ -402,7 +402,7 @@ end);
 
 InstallGlobalFunction(ReadDigraphs,
 function(arg...)
-  local nr, decoder, name, file, i, next, out;
+  local nr, decoder, name, file, i, next, out, line;
 
   # defaults
   nr      := infinity;
@@ -474,8 +474,12 @@ function(arg...)
 
   out := [];
   if NameFunction(decoder) in WholeFileDecoders then
-    Add(out, decoder(IO_ReadUntilEOF(file)));
-    next := IO_Nothing;
+    line := IO_ReadUntilEOF(file);
+    if IsString(arg[1]) then
+      IO_Close(file);
+    fi;
+    Add(out, decoder(line));
+    return out;
   else
     next := decoder(file);
   fi;
@@ -495,7 +499,7 @@ end);
 InstallGlobalFunction(WriteDigraphs,
 function(arg...)
   local name, digraphs, encoder, mode, splitname, compext, g6sum, s6sum, v, e,
-        dg6sum, ds6sum, file, D, i;
+  dg6sum, ds6sum, file, i, D, oldOnBreak, CloseAndCleanup, OnBreakWithCleanup;
 
   # defaults
   encoder := fail;
@@ -606,6 +610,9 @@ function(arg...)
     fi;
     file := DigraphFile(name, encoder, mode);
     if NameFunction(file!.coder) in WholeFileEncoders and file!.mode <> "w" then
+      if IsString(arg[1]) then
+        IO_Close(file);
+      fi;
       ErrorNoReturn(NameFunction(file!.coder), " is a whole file ",
                     "encoder and so the argument <mode> must be \"w\".");
     fi;
@@ -619,8 +626,21 @@ function(arg...)
     fi;
   fi;
 
-  encoder := file!.coder;
+  oldOnBreak := OnBreak;
+  CloseAndCleanup := function()
+    if IsString(arg[1]) then
+      IO_Close(file);
+    fi;
+    OnBreak := oldOnBreak;
+  end;
+
+  OnBreakWithCleanup := function()
+    CloseAndCleanup();
+    OnBreak();
+  end;
+  OnBreak := OnBreakWithCleanup;
 
+  encoder := file!.coder;
   if NameFunction(encoder) in WholeFileEncoders then
     if Length(digraphs) > 1 then
       Info(InfoWarning, 1, "the encoder ", NameFunction(encoder),
@@ -634,10 +654,7 @@ function(arg...)
     od;
   fi;
 
-  if IsString(arg[1]) then
-    IO_Close(file);
-  fi;
-
+  CloseAndCleanup();
   return IO_OK;
 end);
 
@@ -2020,16 +2037,7 @@ end);
 # 4. Encoders
 ################################################################################
 InstallMethod(WriteDIMACSDigraph, "for a digraph", [IsString, IsDigraph],
-function(name, D)
-  local file;
-  file := IO_CompressedFile(UserHomeExpand(name), "w");
-  if file = fail then
-    ErrorNoReturn("cannot open the file given as the 1st argument <name>,");
-  fi;
-  IO_Write(file, DIMACSString(D));
-  IO_Close(file);
-  return IO_OK;
-end);
+              {name, D} -> WriteDigraphs(name, D, DIMACSString, "w"));
 
 InstallMethod(DIMACSString, "for a digraph", [IsDigraph],
 function(D)

tst/standard/io.tst

@@ -12,6 +12,8 @@ gap> LoadPackage("digraphs", false);;
 
 #
 gap> DIGRAPHS_StartTest();
+gap> files := ShallowCopy(IO.OpenFiles);;
+gap> oldOnBreak := OnBreak;;
 
 #  DigraphFromGraph6String and Graph6String
 gap> DigraphFromGraph6String("?");
@@ -276,6 +278,11 @@ gap> gr = ReadDigraphs(filename);
 true
 gap> gr[3] := Digraph([[1, 2], [1, 2]]);
 <immutable digraph with 2 vertices, 4 edges>
+gap> WriteDigraphs(filename, Digraph([[2], []]), Graph6String);
+Error, the argument <D> must be a symmetric digraph with no loops or multiple \
+edges,
+gap> OnBreak := oldOnBreak;;
+gap> IO_Close(IO.OpenFiles[Length(IO.OpenFiles)]);;
 gap> filename := Concatenation(DIGRAPHS_Dir(), "/tst/out/test.s6.bz2");;
 gap> WriteDigraphs(filename, gr, "w");
 IO_OK
@@ -676,6 +683,8 @@ Error, no method found! For debugging hints type ?Recovery from NoMethodFound
 Error, no 1st choice method found for `WriteDIMACSDigraph' on 2 arguments
 gap> WriteDIMACSDigraph("file", ChainDigraph(2));
 Error, the argument <D> must be a symmetric digraph,
+gap> OnBreak := oldOnBreak;;
+gap> IO_Close(IO.OpenFiles[Length(IO.OpenFiles)]);;
 gap> WriteDIMACSDigraph(filename, gr);
 Error, cannot open the file given as the 1st argument <name>,
 gap> filename := "tmp.gz";;
@@ -892,6 +901,9 @@ gap> filename := Concatenation(DIGRAPHS_Dir(), "/tst/out/more.dimacs");;
 gap> WriteDigraphs(filename, gr, "w");;
 gap> ReadDigraphs(filename)[1] = gr;
 true
+gap> WriteDigraphs(filename, gr);
+Error, DIMACSString is a whole file encoder and so the argument <mode> must be\
+ "w".
 gap> DigraphVertexLabels(gr) = [1 .. 3];
 true
 
@@ -950,6 +962,10 @@ gap> DreadnautString(1);
 Error, the first argument <D> must be a digraph
 gap> DreadnautString(D, 1);
 Error, the second argument <partition> must be a list
+gap> WriteDigraphs(filename, Digraph([]), "w");
+Error, the argument <D> must be a digraph with at least one vertex
+gap> OnBreak := oldOnBreak;;
+gap> IO_Close(IO.OpenFiles[Length(IO.OpenFiles)]);;
 gap> WriteDigraphs(filename, D, "w");
 IO_OK
 
@@ -1045,6 +1061,13 @@ gap> DigraphFromDreadnautString("$=1n3dg\n1 : 2 3.\nf=[1:\n:]");
 Error, Invalid range 1 : ':' in partition specification (line 3)
 gap> DigraphFromDreadnautString("$=1n3dg\n1 : 2 3.\nf=[1\nf=2");
 Error, Unexpected character 'f' in partition specification (line 4)
+gap> IO_Close(file);;
+
+# Ensure all files introduced by tests are closed
+gap> IO.OpenFiles = files;
+true
+gap> OnBreak = oldOnBreak;
+true
 
 #  DIGRAPHS_UnbindVariables
 gap> Unbind(D);
@@ -1066,6 +1089,7 @@ gap> Unbind(rdgr);
 gap> Unbind(read);
 gap> Unbind(str);
 gap> Unbind(x);
+gap> Unbind(files);
 
 #
 gap> DIGRAPHS_StopTest();