Skip to content
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

Fix absolute path for test on macos #4139

Merged
merged 1 commit into from
Nov 6, 2020
Merged

Fix absolute path for test on macos #4139

merged 1 commit into from
Nov 6, 2020

Conversation

anandrkskd
Copy link
Contributor

@anandrkskd anandrkskd commented Oct 23, 2020 •
edited

Signed-off-by: anandrkskd anandrkskd@gmail.com

What type of PR is this?
/kind failing-test

What does does this PR do / why we need it:
This PR will fix mac context related issue for test.
Which issue(s) this PR fixes:

Fixes #3797

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

@anandrkskd anandrkskd added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Oct 23, 2020
@openshift-ci-robot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@anandrkskd anandrkskd removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Nov 2, 2020
@anandrkskd anandrkskd marked this pull request as ready for review November 2, 2020 09:59
@anandrkskd
Copy link
Contributor Author

/retest

girishramnani
girishramnani reviewed Nov 4, 2020
View reviewed changes
tests/integration/component.go
@@ -308,8 +308,13 @@ func componentTests(args ...string) {
relativeContext := fmt.Sprintf("..%c%s", filepath.Separator, filepath.Base(commonVar.Context))
fmt.Printf("relativeContext = %#v\n", relativeContext)

helper.CmdShouldPass("odo", append(args, "create", "--s2i", "java:8", "sb-jar-test", "--project",
commonVar.Project, "--binary", filepath.Join(commonVar.Context, "sb.jar"), "--context", relativeContext)...)
if runtime.GOOS == "darwin" {
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the difference between these two conditions?

Copy link
Contributor

Choose a reason for hiding this comment

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

helper.CmdShouldPass("odo", append(args, "create", "--s2i", "java:8", "sb-jar-test", "--project",
					commonVar.Project, "--binary", filepath.Join(commonVar.Context, "sb.jar"), "--context", relativeContext)...)

and

helper.CmdShouldPass("odo", append(args, "create", "--s2i", "java:8", "sb-jar-test", "--project",
					commonVar.Project, "--binary", filepath.Join(commonVar.Context, "sb.jar"), "--context", relativeContext)...)

Copy link
Contributor Author

@anandrkskd anandrkskd Nov 4, 2020 •
edited

Choose a reason for hiding this comment

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

For mac some how the test are using /private/var/.../afaf(random dir) to run test which is not the case for linux and windows, Changing the context passing according to working dir w.r.t test solves the issue.
I think I some how missed while saving and pushing my changes, i have updated the changes,
the change is like this

helper.CmdShouldPass("odo", append(args, "create", "--s2i", "java:8", "sb-jar-test", "--project",
					commonVar.Project, "--binary", filepath.Join("/private",commonVar.Context, "sb.jar"), "--context", relativeContext)...)

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry but before there wasn't a "/private` in the filepath.Join, and I would suggest not force pushing the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood

Copy link
Contributor

Choose a reason for hiding this comment

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

/approve

@anandrkskd
Fix absolute path for test on macos
6d2fbaa 
Signed-off-by: anandrkskd <anandrkskd@gmail.com>
@codecov
Copy link

codecov bot commented Nov 4, 2020 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@436015d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #4139   +/-   ##
=========================================
  Coverage          ?   42.28%           
=========================================
  Files             ?      154           
  Lines             ?    13074           
  Branches          ?        0           
=========================================
  Hits              ?     5528           
  Misses            ?     6954           
  Partials          ?      592
Impacted Files Coverage Δ
pkg/devfile/adapters/common/types.go 50.00% <0.00%> (ø)
pkg/occlient/pods.go 0.00% <0.00%> (ø)
pkg/odo/cli/env/unset.go 0.00% <0.00%> (ø)
pkg/testingutil/deploymentconfigs.go 0.00% <0.00%> (ø)
pkg/lclient/generators.go 100.00% <0.00%> (ø)
pkg/devfile/parser/types.go 64.70% <0.00%> (ø)
pkg/occlient/occlient.go 48.40% <0.00%> (ø)
pkg/testingutil/pods.go 0.00% <0.00%> (ø)
pkg/occlient/fakeclient.go 74.19% <0.00%> (ø)
pkg/odo/cli/service/create.go 9.87% <0.00%> (ø)
... and 144 more

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 436015d...6d2fbaa. Read the comment docs.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: girishramnani

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Nov 4, 2020
@prietyc123
Copy link
Contributor

I verified the change locally on macOS and the test runs successfully.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Nov 6, 2020
@openshift-merge-robot openshift-merge-robot merged commit 30d9478 into redhat-developer:master Nov 6, 2020
@anandrkskd anandrkskd deleted the test-fix-absolute-path branch September 28, 2021 04:53
@cdrage cdrage mentioned this pull request Jul 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@girishramnani girishramnani girishramnani left review comments

@adisky adisky Awaiting requested review from adisky

@maysunfaisal maysunfaisal Awaiting requested review from maysunfaisal

Assignees

@prietyc123 prietyc123

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Binary component create fails if path is relative and includes ../
5 participants
@anandrkskd @openshift-ci-robot @prietyc123 @girishramnani @openshift-merge-robot