S3 blobstore driver

[aduffeck]

Note: This draft pr is not about the storage driver (which is still WIP), it's merely meant to trigger a discussion about unit testing in general.

I'm working on a new storage driver and would like to write unit tests for it because I think that's gonna improve the code and its stability.

I'm aware that testing and the tools and frameworks around it can be controversial so I'd love to hear what you think before I dive into it too deep.

I believe that using assertion libraries and testing frameworks on top of plain go tests can greatly improve the readability of the tests and turn them into some kind of documentation which can help a lot to understand how the tested code is supposed to work. I chose the following libraries as a starting point:

• Ginkgo/Gomega for the testing framework and the assertion library
• Testify/mockery for the mocking library and generator

While there are many alternatives or you can write your tests without using any of those the ones I picked are very widely used and allow for writing very clean and expressive tests.

Here's a simple example from this draft pr showing the grouping and structuring capabilities of Ginkgo combined with the Gomega assertions and Testify mocks:

var _ = Describe("File uploads", func() {

	[snip]

	Context("with insufficient permissions", func() {
		BeforeEach(func() {
			permissions.On("HasPermission", mock.Anything, mock.Anything, mock.Anything).Return(false, nil)
		})

		Describe("InitiateUpload", func() {
			It("fails", func() {
				_, err := fs.InitiateUpload(ctx, ref, 10, map[string]string{})
				Expect(err).To(MatchError("error: permission denied: root/foo"))
			})
		})
	})
})

Please let me know what you think about that.

/cc @labkode @ishank011 @butonic @refs

[refs]

I'm all in for introducing BDD in the test suite. It won't bloat the binary size as test dependencies are not included in the final artifact and it will allow us to have a green/red test environment.

[butonic]

cc @individual-it @phil-davis

[individual-it]

I'm suggesting to use

1. the existing API tests that come from owncloud, they should show how the driver performs in various scenarios and using the expected-to-fail files they would show what still needs to be done - basically BDD. We do that already with the OWNCLOUD storage driver and the OCIS storage driver
2. create GRPC based tests as shown in the POC in https://github.com/cs3org/reva/tree/master/grpc-tests Ideally the same tests could also run against other storage drivers (I assume they suppose all to be compatible and using the same interfaces)

Please correct me if I assume the storage driver on a wrong level of the software

[aduffeck]

Oh, I agree. I'm not saying that the existing integration tests shouldn't be used. I'm rather suggesting to add another layer of tests at the unit level. Those bring several advantages like

• much quicker and more direct feedback as they run in seconds on the developer machine and on a lower level
• better code coverage as it's much easier to test corner cases etc.
• TDD. Unit tests enable developers to follow the TDD style which - in my opinion - leads to cleaner, clearer and more stable code as you are forced to think about the design of the software before starting implementing it
• living documentation. As mentioned above a good unit test suite is great for new people to understand how the code is supposed to work, for example

[dragotin]

That would be a great move from my high level POV. @labkode what is your opinion?

[butonic]

@labkode I assume the storage provider testsuite needs an integration test like #1048. We can use the unit tests introduced in this PR for the other storage drivers, though. Any objections to adding Gingko and Testify?

[labkode]

Hi, first of all, sorry for the delay, busy week last one. I happily support extending the testing suite in this direction. I have checked the licenses for both projects and are compatible with the Apache 2.0 we have for Reva.

Despite of introducing these tools for making testing more Dev friendly, another important area is where to introducing testing.

Having unit tests for the storage implementations is important to describe the inner behaviour, but having them on the interface level is of more importance and it defines the behaviour of the system as a whole.

Having the tests at the interface level allows developers to contribute with plugins with more confidence. The current tests that bring some degree of confidence are the webdav tests contributed by oc.

This is a typical flow:
webdav > cs3apis > storage interface > storage plugin (s3, eos)

The CS3APIs are defining the behaviour of the system (@butonic pointed to some work) so I'd be keen to introduce them at that level rather that at the Go interface.

Let me know your thoughts.

[aduffeck]

Hi @labkode. No worries, I understand you all were quite busy lately.

In my opinion this is not an either-or kind of thing. Integration (or functional) tests at the interface level are important and great for validating that the software as a whole works as specified. They make sure that the different units work well together and that the end results of the actions of public interfaces are what the user expects.

So having such a test suite that can be used against all drivers to ensure that they all fulfill the specification would be really great. It can't, however, replace testing at the unit level with all the benefits I tried to describe above I think.

[labkode]

Hi @aduffeck , I wasn't suggesting neither to choose one or another. Both are important, what I meant is that starting adding the tests at the interface level will pay off when adding more implementations to raise the confidence.
I see that you have already added the unit tests in #1428, looks great.

[aduffeck]

Awesome, sounds like we're on the same page then 👍

[aduffeck]

Closing as we seem to have reached a conclusion. Thanks again