Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Do not depend on GOPATH to locate test data #122

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

honnix
Copy link
Member

@honnix honnix commented Sep 2, 2020

TL;DR

GOPATH is not mandatory to have now and the source code may not be stored under GOPATH.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Use os.Getwd() to get current directory and then join path.

Tracking Issue

flyteorg/flyte#498

Follow-up issue

GOPATH is not mandatory to have now and the source code may not be
stored under GOPATH.
@codecov-commenter
Copy link

Codecov Report

Merging #122 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
- Coverage   62.39%   62.34%   -0.05%     
==========================================
  Files         104      104              
  Lines        7752     7758       +6     
==========================================
  Hits         4837     4837              
- Misses       2345     2351       +6     
  Partials      570      570              
Flag Coverage Δ
#unittests 62.34% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/repositories/config/migrations.go 0.00% <0.00%> (ø)

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 bf159e5...54c3cf9. Read the comment docs.

}

configAccessor := viper.NewAccessor(config.Options{
SearchPaths: searchPaths,
SearchPaths: []string{filepath.Join(pwd, "../testdata", fileName)},
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you so much, but what do you think about moving the test yaml to this folder?

Copy link
Contributor

Choose a reason for hiding this comment

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

does not need to part of this change, we can file a bug and handle it as a follow up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I prefer that in a separated PR because it is not really part of this fix.

@kumare3 kumare3 self-requested a review September 2, 2020 22:59
Copy link
Contributor

@kumare3 kumare3 left a comment

Choose a reason for hiding this comment

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

lgtm

@honnix honnix merged commit 48c93d6 into master Sep 3, 2020
@honnix honnix deleted the fix-go-path-dependent-tests branch September 3, 2020 10:36
schottra added a commit that referenced this pull request Sep 8, 2020
* master:
  Gcs remote data (#121)
  Do not depend on GOPATH to locate test data (#122)
  Add index to optimize for list task executions for node execution (#120)
  Grpc health checking (#118)
  Allow random cluster selection when no override (#117)
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
GOPATH is not mandatory to have now and the source code may not be
stored under GOPATH.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants