Skip to content

Add tests to PipelineGitLSRemote #40

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 18, 2018
Merged

Add tests to PipelineGitLSRemote #40

merged 2 commits into from
Jul 18, 2018

Conversation

al3rez
Copy link
Contributor

@al3rez al3rez commented Jul 18, 2018 •
edited
Loading

Tests that check whether PipelineGitLSRemote works properly.

  • Invalid Data
  • Invalid Access
  • Success

@al3rez
Copy link
Contributor Author

al3rez commented Jul 18, 2018

@michelvocks are you ok with this type of tests?

michelvocks
michelvocks reviewed Jul 18, 2018
View reviewed changes
Copy link
Member

@michelvocks michelvocks left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks a lot @azbshiri for your contribution. 🤗
See my single comment but the rest looks really good to me.

handlers/pipeline_test.go Outdated
"github.com/gaia-pipeline/gaia/store"
hclog "github.com/hashicorp/go-hclog"
"github.com/labstack/echo"
"github.com/stretchr/testify/assert"
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if you could remove this dependency. I know assert from other projects (docker for example) and they had at the end huge problems with it. It seems like it's not 100% reliable. 😕

I know it's more overhead and makes the tests a bit more ugly but at least we are 100% using the standard library which I would like to stick if it's somehow possible. 🤗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@codecov-io
Copy link

Codecov Report

Merging #40 into master will increase coverage by 0.81%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   46.68%   47.49%   +0.81%     
==========================================
  Files          13       13              
  Lines         979      979              
==========================================
+ Hits          457      465       +8     
+ Misses        472      464       -8     
  Partials       50       50
Impacted Files Coverage Δ
handlers/pipeline.go 9.75% <0%> (+9.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e99da6...81a34b5. Read the comment docs.

@michelvocks
Copy link
Member

LGTM. Thanks a lot 🤗 ❤️

@michelvocks michelvocks merged commit 08f4a19 into gaia-pipeline:master Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michelvocks michelvocks michelvocks left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@al3rez @codecov-io @michelvocks