Testing/round trip go

.github/workflows/bench_pr.yml

@@ -6,10 +6,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.15
+    - name: Set up Go 1.16
       uses: actions/setup-go@v2
       with:
-        go-version: 1.15
+        go-version: 1.16
       id: go
 
     - name: Check out code 

.github/workflows/bench_push.yml

@@ -5,10 +5,10 @@ jobs:
     name: Benchmark (push)
     runs-on: ubuntu-latest
     steps:
-    - name: Set up Go 1.15
+    - name: Set up Go 1.16
       uses: actions/setup-go@v2
       with:
-        go-version: 1.15
+        go-version: 1.16
       id: go
 
     - name: Check out code 

.github/workflows/go.yml

@@ -7,10 +7,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.15
+    - name: Set up Go 1.16
       uses: actions/setup-go@v2
       with:
-        go-version: 1.15
+        go-version: 1.16
       id: go
 
     - name: Check out code 

go.mod

@@ -1,6 +1,6 @@
 module github.com/suyashkumar/dicom
 
-go 1.13
+go 1.16
 
 require (
 	github.com/golang/mock v1.4.4

go.sum

@@ -2,12 +2,17 @@ github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc=
 github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
 github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 h1:VklqNMn3ovrHsnt90PveolxSbWFaJdECFbxSq0Mqo2M=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/net v0.0.0-20190311183353-d8887717615a h1:oWX7TPOiFAMXLq8o0ikBYfCJVlRHBcsciT5bXOrH628=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a h1:1BGLXjeY4akVXGgbC9HugT3Jv3hCI0z56oJR5vAMgBU=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/tools v0.0.0-20190425150028-36563e24a262 h1:qsl9y/CJx34tuA7QCPNp86JNJe4spst6Ff8MjvPUdPg=
 golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

parse_test.go

@@ -4,79 +4,82 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/suyashkumar/dicom/pkg/frame"
+	"github.com/suyashkumar/dicom/pkg/tag"
 	"image/jpeg"
-	"io/ioutil"
+	"io"
 	"os"
-	"strings"
 	"testing"
 
-	"github.com/suyashkumar/dicom/pkg/tag"
-
-	"github.com/suyashkumar/dicom/pkg/frame"
-
 	"github.com/suyashkumar/dicom"
+	"github.com/suyashkumar/dicom/pkg/dcmtest"
 )
 
-// TestParse is an end-to-end sanity check over DICOMs in testdata/. Currently it only checks that no error is returned
-// when parsing the files.
+// TestParse is an end-to-end sanity check over DICOMs in /pkg/dcmtest/data/. Currently
+// it only checks that no error is returned when parsing the files.
 func TestParse(t *testing.T) {
-	files, err := ioutil.ReadDir("./testdata")
-	if err != nil {
-		t.Fatalf("unable to read testdata/: %v", err)
-	}
-	for _, f := range files {
-		if !f.IsDir() && strings.HasSuffix(f.Name(), ".dcm") {
-			t.Run(f.Name(), func(t *testing.T) {
-				dcm, err := os.Open("./testdata/" + f.Name())
-				if err != nil {
-					t.Errorf("Unable to open %s. Error: %v", f.Name(), err)
-				}
-				defer dcm.Close()
-				info, err := dcm.Stat()
-				if err != nil {
-					t.Errorf("Unable to stat %s. Error: %v", f.Name(), err)
-				}
-				_, err = dicom.Parse(dcm, info.Size(), nil)
-				if err != nil {
-					t.Errorf("dicom.Parse(%s) unexpected error: %v", f.Name(), err)
-				}
-			})
+	// This will walk all of our test data and try parsing the Dataset, so we can simply
+	// report if we get passed an error.
+	dcmtest.WalkIncludedFS(t, func(t *testing.T, tc dcmtest.FSTestCase, setupErr error) {
+		if setupErr != nil {
+			t.Fatalf("setup error: %v", setupErr)
 		}
-	}
+	})
 }
 
-// BenchmarkParse runs sanity benchmarks over the sample files in testdata.
+// BenchmarkParse runs sanity benchmarks over the sample files in /pkg/dcmtest/data/.
 func BenchmarkParse(b *testing.B) {
-	files, err := ioutil.ReadDir("./testdata")
-	if err != nil {
-		b.Fatalf("unable to read testdata/: %v", err)
-	}
-	for _, f := range files {
-		if !f.IsDir() && strings.HasSuffix(f.Name(), ".dcm") {
-			b.Run(f.Name(), func(b *testing.B) {
-				dcm, err := os.Open("./testdata/" + f.Name())
-				if err != nil {
-					b.Errorf("Unable to open %s. Error: %v", f.Name(), err)
-				}
-				defer dcm.Close()
-
-				data, err := ioutil.ReadAll(dcm)
-				if err != nil {
-					b.Errorf("Unable to read file into memory for benchmark: %v", err)
-				}
-
-				b.ResetTimer()
-				for i := 0; i < b.N; i++ {
-					_, _ = dicom.Parse(bytes.NewBuffer(data), int64(len(data)), nil)
-				}
-			})
+	// This will walk all of our test data and run a sub-test for each one.
+	dcmtest.BenchIncludedFS(b, func(b *testing.B, tc dcmtest.FSTestCase, setupErr error) {
+		// The test helper will have already parsed the file once, so we will use that
+		// as validation that the dataset is good.
+		if setupErr != nil {
+			b.Fatalf("setup error: %v", setupErr)
 		}
-	}
+
+		dcmFile, err := tc.OpenDCMFile()
+		if err != nil {
+			b.Fatalf("error opening dcm file: %v", err)
+		}
+		defer dcmFile.Close()
+
+		// Extract the binary data.
+		binData, err := io.ReadAll(dcmFile)
+		if err != nil {
+			b.Fatalf("error reading dcm file: %v", err)
+		}
+
+		b.ResetTimer()
+		for i := 0; i < b.N; i++ {
+			// Read from a fresh buffer each iteration.
+			buffer := bytes.NewBuffer(binData)
+
+			// Parse the dataset.
+			dataset, err := dicom.Parse(buffer, tc.DCMStat.Size(), nil)
+			if err != nil {
+				b.Fatalf("error parsing dataset on repetition %v: %v", i, err)
+			}
+
+			// Make sure we got sane results.
+			if len(dataset.Elements) == 0 {
+				b.Fatalf("no elements were parsed on repetition %v", i)
+			}
+
+			if len(dataset.Elements) != len(tc.Dataset.Elements) {
+				b.Fatalf(
+					"element count mismatch: expected %v, got %v on repetition %v",
+					len(tc.Dataset.Elements),
+					len(dataset.Elements),
+					i,
+				)
+			}
+		}
+	})
 }
 
 func Example_readFile() {
 	// See also: dicom.Parse, which uses a more generic io.Reader API.
-	dataset, _ := dicom.ParseFile("testdata/1.dcm", nil)
+	dataset, _ := dicom.ParseFile("./pkg/dcmtest/data/1.dcm", nil)
 
 	// Dataset will nicely print the DICOM dataset data out of the box.
 	fmt.Println(dataset)
@@ -99,12 +102,12 @@ func Example_streamingFrames() {
 		}
 	}()
 
-	dataset, _ := dicom.ParseFile("testdata/1.dcm", frameChan)
+	dataset, _ := dicom.ParseFile("./pkg/dcmtest/data/1.dcm", frameChan)
 	fmt.Println(dataset) // The full dataset
 }
 
 func Example_getImageFrames() {
-	dataset, _ := dicom.ParseFile("testdata/1.dcm", nil)
+	dataset, _ := dicom.ParseFile("./pkg/dcmtest/data/1.dcm", nil)
 	pixelDataElement, _ := dataset.FindElementByTag(tag.PixelData)
 	pixelDataInfo := dicom.MustGetPixelDataInfo(pixelDataElement.Value)
 	for i, fr := range pixelDataInfo.Frames {

pkg/dcmtest/data.go

@@ -0,0 +1,18 @@
+package dcmtest
+
+import (
+	"embed"
+)
+
+// IncludedFS uses the embed package to read all of the test files in /pkg/dcmtest/data
+// into an embed.FS for testing. This allows us to keep the files in memory without
+// reading them from disk each time they are needed, or worrying about relative import
+// paths.
+//
+// WARNING
+//
+// This var should only be used for testing purposes. It contains a number of full DICOM
+// files that could significantly bloat a binary's size if included in production code.
+//
+//go:embed **/*.dcm
+var IncludedFS embed.FS

pkg/dcmtest/doc.go

@@ -0,0 +1,9 @@
+// Package dcmtest contains helper methods and functions for testing against dicom
+// files.
+//
+// WARNING
+//
+// This module should only be used by test files or test helper packages to be run by
+// `gotest`, as it included an embedded dataset which will lead to significant binary
+// size bloat when imported into production code.
+package dcmtest

pkg/dcmtest/examples_test.go

@@ -0,0 +1,32 @@
+package dcmtest_test
+
+import (
+	"github.com/suyashkumar/dicom"
+	"github.com/suyashkumar/dicom/pkg/dcmtest"
+	"testing"
+)
+
+func MyDatasetFunc(dataset dicom.Dataset) error {
+	return nil
+}
+
+func ExampleWalkIncludedFS() {
+	// For this example we will make a mock *testing.T value. Normally this would be
+	// passed into your testing function automatically by `go test`.
+	t := new(testing.T)
+
+	// Each file in dcmtest.IncludedFS will be run in it's own subtest automatically.
+	dcmtest.WalkIncludedFS(t, func(t *testing.T, tc dcmtest.FSTestCase, setupErr error) {
+		// On a setup error, we want to report it. setupErr is not reported to t
+		// automatically, and is left to the caller to handle.
+		if setupErr != nil {
+			t.Fatal("setup error:", setupErr)
+		}
+
+		// Pass the pared dataset to the function we actually want to test
+		err := MyDatasetFunc(tc.Dataset)
+		if err != nil {
+			t.Error("MyDatasetFunc error:", err)
+		}
+	})
+}