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 integration tests with skip lists for failing tests #33

Merged
merged 12 commits into from
May 29, 2020
22 changes: 11 additions & 11 deletions ion/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ func TestNonEquivalency(t *testing.T) {
})
}

// Execute round trip testing for BinaryWriter. Create a BinaryWriter, using the BinaryWriter create
// a TextWriter and back to BinaryWriter again. Validate that the first and last Writers are equal.
// Execute round trip testing for the BinaryWriter by creating a BinaryWriter and re-encoding it to text.
// From the output of the writers, construct two readers and verify the equivalency of both readers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Re-encodes the provided file as binary Ion, reads that binary Ion and writes it as text Ion,
// then constructs Readers over the binary and text encodings to verify that the streams 
// are equivalent.

func testBinaryRoundTrip(t *testing.T, fp string) {
fileBytes := loadFile(t, fp)

Expand All @@ -369,8 +369,8 @@ func testBinaryRoundTrip(t *testing.T, fp string) {
}
}

// Execute round trip testing for TextWriter. Create a TextWriter, using the TextWriter creat a
// BinaryWriter and back to TextWriter again. Validate that the first and last Writers are equal.
// Execute round trip testing for the TextWriter by creating a TextWriter and re-encoding it to binary.
// From the output of the writers, construct two readers and verify the equivalency of both readers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Re-encodes the provided file as text Ion, reads that text Ion and writes it as binary Ion, 
// then constructs Readers over the text and binary encodings to verify that the streams 
// are equivalent.

func testTextRoundTrip(t *testing.T, fp string) {
fileBytes := loadFile(t, fp)

Expand All @@ -395,7 +395,7 @@ func testTextRoundTrip(t *testing.T, fp string) {
}
}

// Create a TextWriter from data parameter. Return the writer and string builder containing writer's contents
// Create a TextWriter from data parameter and return the string builder containing writer's contents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're not returning the TextWriter, it's an implementation detail.

// Re-encode the provided Ion data as a text Ion string.

func encodeAsTextIon(t *testing.T, data string) strings.Builder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Golang allows you to use the string data type to mean "arbitrary bytes", but I don't think we should use it that way. When I first read this function I was surprised because I thought if data was a string, it must already be text Ion so there'd be no need to encode it as text Ion. Only when I went and looked at the callsite did I realize that we were taking a binary buffer and calling String() on it to get a string instance.

We should make sure we're only using string to refer to text throughout the APIs. If this is a quick change, you can include it in the PR. Otherwise, please open an issue so we don't forget to address it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it in this test, create #48 to check throughout the project.

reader := NewReader(strings.NewReader(data))
str := strings.Builder{}
Expand All @@ -408,17 +408,17 @@ func encodeAsTextIon(t *testing.T, data string) strings.Builder {
return str
}

// Create a BinaryWriter from data parameter. Return the writer and buffer containing writer's contents
// Create a BinaryWriter from data parameter and return the buffer containing writer's contents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we're not returning the BinaryWriter, it's an implementation detail.

// Re-encode the provided Ion data as a binary Ion buffer.

func encodeAsBinaryIon(t *testing.T, data []byte) bytes.Buffer {
reader := NewReader(bytes.NewReader(data))
buf2 := bytes.Buffer{}
binWriter2 := NewBinaryWriter(&buf2)
writeToWriterFromReader(t, reader, binWriter2)
err := binWriter2.Finish()
buf := bytes.Buffer{}
binWriter := NewBinaryWriter(&buf)
writeToWriterFromReader(t, reader, binWriter)
err := binWriter.Finish()
if err != nil {
t.Fatal(err)
}
return buf2
return buf
}

// Execute loading malformed Ion values into a Reader and validate the Reader.
Expand Down