drone-plugins / drone-s3

Drone plugin for publishing artifacts to Amazon S3
http://plugins.drone.io/drone-plugins/drone-s3
Apache License 2.0
36 stars 64 forks source link

WIP: Unit tests for drone-s3 plugin #3

Closed RomanSaveljev closed 8 years ago

RomanSaveljev commented 8 years ago

This is a running PR, which is about to bring some unit tests to cover the plugin's functionality. Couple of tests fail due to (subjective) bugs in the plugin, so some merging/rebasing should follow.

It is put up early for awareness and discussion

msteinert commented 8 years ago

@RomanSaveljev are you still interested in this PR? I'm willing to review if you want to continue working on it.

I wonder if something like fake-s3 running in a service container might make sense rather than running it against a live S3 account. There are a number of automated builds on Docker Hub that might be suitable.

bradrydzewski commented 8 years ago

fake-s3 in a service container sounds like the way to go.

alternatively, if we want something a bit more simple to ensure commands are being executed with the expected parameters, and mock the responses, we can use https://github.com/tsuru/commandmocker

RomanSaveljev commented 8 years ago

@msteinert I am not using a live S3 account in tests. The aws binary is replaced and acts as a mock. I am a bit hesitant of this PR, because there is quite few technologies involved (bats, perl, go..). On the other hand some tests is better than no tests.

Up to you..

bradrydzewski commented 8 years ago

@RomanSaveljev thanks for taking the time to write some tests. I think we could reduce the amount of custom scripting by using https://github.com/tsuru/commandmocker (or better yet fake-s3)

Command mocker does something similar (I think) to what you are trying to achieve. The below code handles path overriding (and cleanup) to ensure that a command returns the desired exit code and message:

    message := "cannot connect to aws"
    path, err := commandmocker.Error("aws", message, 1)
    if err != nil {
        t.Fatal(err)
    }
    defer commandmocker.Remove(path)

I do appreciate the ideas, and would love to see the PR re-worked to use some of the existing tooling out there. Thanks!

RomanSaveljev commented 8 years ago

@bradrydzewski Yes, agreed. commandmocker definitely looks better for those used to code Go