-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
integration: test case for HOME #1072
integration: test case for HOME #1072
Conversation
This is the same bug as #1082 |
@greut Thank you for looking into this and submitting this PR. I think the culprit here is In this func at L106, we return immediately if
Since this is a critical bug, we would like to get this in before next release. Let us know if you need any help finishing this PR. Thanks |
Iirc test is posix, not bash.
…On Thu, Feb 27, 2020 at 20:49, Tejal Desai ***@***.***> wrote:
@tejal29 commented on this pull request.
---------------------------------------------------------------
In [integration/dockerfiles/Dockerfile_test_user_home](#1072 (comment)):
> +# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+FROM debian:9.11
+
+# default values for the root user.
+RUN test "/root" = "$HOME" && \
please remove bash test .
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#1072?email_source=notifications&email_token=AAAAK3EHUO3JMTRAZ2JELCLRFAKLDA5CNFSM4KZ36UKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCXH7PBY#pullrequestreview-365950855), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAAK3FXU2B7ZPXD5RAHIJ3RFAKLDANCNFSM4KZ36UKA).
|
Ack. I meant assertions inside |
@greut would you like to also add the fix to the code so your test pass? Let me know if you need more help. |
Running USER didn't reset HOME which had to be explicitly set. Closes GoogleContainerTools#1072 Signed-off-by: Yoan Blanc <[email protected]>
fa0f9c1
to
403c042
Compare
|
3c34c0b
to
dbabc6c
Compare
@tejal29 |
645a3a8
to
07508f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explanation for a corner case.
Thanks @greut I am going to rebase your PR and resolve conflicts. Hopefully your branch will be green now. |
Signed-off-by: Yoan Blanc <[email protected]>
Running USER didn't reset HOME which had to be explicitly set. Closes GoogleContainerTools#1082 Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
c7d3438
to
9e83210
Compare
👍 |
Now we are seeing the error where
|
@tejal29 I would have preferred a merge over that rebase to be frank. |
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
Signed-off-by: Yoan Blanc <[email protected]>
wooooooooooohooo 😜 |
Description
According to #824, #1097 and #995, the
$HOME
variable should be set and correct when usingUSER
.Is it a limitation of the design? If not, do you see a way to get this fixed? If yes, any guidance would be appreciated to give a hand in the area.
Cheers
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Reviewer Notes
Release Notes
Describe any changes here so maintainer can include it in the release notes, or delete this block.