Skip to content

Commit d7759d7

Browse files
emdash-iegroakland
andcommitted
Add assertion on tag type
As pointed out in code review, since the tests only checked that a tag has decoded, it was possible for a tag to decode successfully but change tag type, e.g. if two types swapped their IDs. This commit updates the code to assert on the tag type and make sure they haven’t. This also exposes the same weakness in the existing tests, in that they don’t exhaustively check that the binary content deserialises to what we’d expect it to. I guess we could change that? Co-authored-by: Grahame Oakland <grahame.oakland@guardian.co.uk>
1 parent bd7929f commit d7759d7

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

scala/src/test/scala/com.gu.contentapi.scala/ThriftRoundTripSpec.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class ThriftRoundTripSpec extends FlatSpec with Matchers {
4040

4141
it should "round-trip TagType enum values" in {
4242
for {
43-
(rawPath, _tagType) <- Seq(
43+
(rawPath, tagType) <- Seq(
4444
"some-contributor-tag.binary.thrift" -> TagType.Contributor,
4545
"some-keyword-tag.binary.thrift" -> TagType.Keyword,
4646
"some-series-tag.binary.thrift" -> TagType.Series,
@@ -55,7 +55,7 @@ class ThriftRoundTripSpec extends FlatSpec with Matchers {
5555
"some-campaign-tag.binary.thrift" -> TagType.Campaign,
5656
)
5757
} yield {
58-
checkRoundTrip(Path.of(rawPath), Tag)
58+
checkRoundTrip(Path.of(rawPath), Tag, (tag: Tag) => tag.`type` shouldBe tagType)
5959
}
6060
}
6161

0 commit comments

Comments
 (0)