Add unit tests for frames.

[AasthaGupta]

This PR includes unit test for the different frame type. It covers test cases for initialization, encoding, decoding and flag setting.

This will increase the code coverage from 0% to 64%.

[OlegDokuka]

@AasthaGupta that is too early to add tests now. The API will be changed, so adding tests at this moment will just make it more challenging for us to break things when we find them less convenient.

At least, I would deffer mergin that until the internal implementation will be stabilized

[nkristek]

First of all, well done and thank you! I have a few suggestions, otherwise very good!

[nkristek]

Instead of checking if the bits are equal OptionSet provides contains that can be used like:

Suggested change

	XCTAssert(payloadFrame.header.flags.rawValue & FrameFlags.payloadFollows.rawValue != 0, "Expected follows flag to be set")
	XCTAssert(payloadFrame.header.flags.rawValue & FrameFlags.payloadComplete.rawValue != 0, "Expected complete flag to be set")
	XCTAssert(payloadFrame.header.flags.rawValue & FrameFlags.payloadNext.rawValue != 0, "Expected next flag to be set")
	XCTAssert(payloadFrame.header.flags.rawValue & FrameFlags.metadata.rawValue != 0, "Expected metadata flag to be set")
	XCTAssert(payloadFrame.header.flags.contains(.payloadFollows), "Expected follows flag to be set")
	XCTAssert(payloadFrame.header.flags.contains(.payloadComplete), "Expected complete flag to be set")
	XCTAssert(payloadFrame.header.flags.contains(.payloadNext), "Expected next flag to be set")
	XCTAssert(payloadFrame.header.flags.contains(.metadata), "Expected metadata flag to be set")

[nkristek]

This is just a suggestion and fine if we don't apply it, but we could also define the enclosing method to be throwing:

Suggested change

	func testPayloadEncoder() {
	let payloadMetadata = "m".data(using: .utf8)!
	let payloadData = "d".data(using: .utf8)!
	let payloadFrame = PayloadFrame(streamId: 1, fragmentsFollow: true, isCompletion: true, isNext: true, payload: Payload(metadata: payloadMetadata, data: payloadData))
	guard var byteBuffer = try? PayloadFrameEncoder().encode(frame: payloadFrame, using: ByteBufferAllocator()) else {
	XCTFail("Expected byte buffer to be not nil")
	return
	}
	XCTAssertEqual(byteBuffer.readableBytes, 11)
	XCTAssertEqual(byteBuffer.readBytes(length: byteBuffer.readableBytes), PayloadFrameTests.bytes)
	}
	func testPayloadEncoder() throws {
	let payloadMetadata = "m".data(using: .utf8)!
	let payloadData = "d".data(using: .utf8)!
	let payloadFrame = PayloadFrame(streamId: 1, fragmentsFollow: true, isCompletion: true, isNext: true, payload: Payload(metadata: payloadMetadata, data: payloadData))
	var byteBuffer = try PayloadFrameEncoder().encode(frame: payloadFrame, using: ByteBufferAllocator())
	XCTAssertEqual(byteBuffer.readableBytes, 11)
	XCTAssertEqual(byteBuffer.readBytes(length: byteBuffer.readableBytes), PayloadFrameTests.bytes)
	}

[nkristek]

@AasthaGupta that is too early to add tests now. The API will be changed, so adding tests at this moment will just make it more challenging for us to break things when we find them less convenient.

At least, I would deffer mergin that until the internal implementation will be stabilized

Frame logic can be tested imho, it shouldn't change that much anymore. I will add some convenience soon that builds on the existing logic.

[OlegDokuka]

@nkristek
Right, the framing logic itself - true. Encoders - I would disagree. As we discussed, we can add more improvements to the existing codecs API. From my personal experience, writing tests for unstabilized API end ups in slowing down the whole development and research process. So when you have to rework the API or try out a new technique, apart from core changes you either have to maintain the tests or you usually comment/remove them entirely since they do not applicable or too hard to change them quickly. I'm fine if that great effort will be merged when we have the core part 90% implemented and stabilized and we know that what we have done unlikely to be changed.

Please, don't take me wrong, tests are inevitable, but I'm pessimistic about merging/doing that work now based on my experience.