This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Add unit test for os.chown
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Claudiu.Popa, berker.peksag, cormac-obrien, ezio.melotti, python-dev, r.david.murray, serhiy.storchaka, vajrasky
Priority: normal Keywords: patch

Created on 2013-12-26 10:28 by vajrasky, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
add_unit_test_os_chown_python27.patch vajrasky, 2013-12-26 10:28 For Python 2.7 review
add_unit_test_os_chown_v2.patch vajrasky, 2014-02-23 13:28 For Python 3.4 review
add_unit_test_os_chown_v3.patch vajrasky, 2014-06-18 15:21 review
add_unit_test_os_chown_v4.patch vajrasky, 2014-06-24 15:49 review
add_unit_test_os_chown_v5.patch vajrasky, 2014-06-26 15:58 review
unittest_os_main.patch cormac-obrien, 2014-11-01 01:57 review
add_unit_test_os_chown_v6.patch vajrasky, 2014-11-01 09:12 review
add_unit_test_os_chown_v7.patch vajrasky, 2014-12-25 09:17 review
Messages (19)
msg206936 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-26 10:28
There is no tests for os.chown except for the invalid input.

Attached the patch to add tests for os.chown. This test has not exercised the keyword `dir_fd` and `follow_symlinks` because `follow_symlinks` (I think) is kinda broken on Windows and I prefer to add unit test for `dir_fd` keyword in different ticket.
msg206937 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-26 10:28
Here is the patch for Python 2.7.
msg211993 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-02-23 13:28
Here is the patch that is considerate towards Windows for Python 3.4. I'll fix the patch for Python 2.7 later.
msg220582 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-14 21:35
Hi, I left a couple of comments on rietveld.
msg220941 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-06-18 15:21
Here is the patch based on Claudiu's comment and Serhiy's commit (http://hg.python.org/cpython/rev/4b187f5aa960).

Claudiu (Thanks!!!), I can not apply two of your suggestions:
"You could use support.import_module here in order to skip the test if pwd can't be imported."
=> It will skip the entire file if I do that which is bad for Windows.

"You could use assertRaisesRegex."
=> I can not because assertRaisesRegex can not be used with "with" statement.
msg220942 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-18 15:25
os.chown is not available on Windows. But maybe pwd can't be built on any Unix system, so not using import_module is okay.

"=> I can not because assertRaisesRegex can not be used with "with" statement."

Not true (see https://docs.python.org/3/library/unittest.html?highlight=assertraisesregex#unittest.TestCase.assertRaisesRegex).
msg221072 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-20 07:55
I think you can skip the entire test class if os.chown is not available. Also, maybe you can move the obtaining of groups and users in setUpClass? 
Also, in 

+        with self.assertRaises(PermissionError) as cx:
+            os.chown(support.TESTFN, uid_1, gid)
+            os.chown(support.TESTFN, uid_2, gid)

if the first os.chown will raise the PermissionError, the second one won't be called, maybe that's not what you intended to do.
msg221471 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-06-24 15:49
Claudiu, I have revamped the test and put it in dedicated test class. Thanks!

1. About assertRaisesRegex, sorry, you're right. We can use it with "with" statement. Prior to this, I used it like this:

with self.assertRaisesRegex(PermissionError,
                                    "Operation not permitted"):
    bla
    bla

Then I realized I have to use it like this:

with self.assertRaisesRegex(PermissionError,
                                    "Operation not permitted") as _:
    bla
    bla

2. About this code, it is deliberate:

+        with self.assertRaises(PermissionError) as cx:
+            os.chown(support.TESTFN, uid_1, gid)
+            os.chown(support.TESTFN, uid_2, gid)

Normal user does not have permission to use chown to change the user of the file. Basically, I need to find the other user other than the user of support.TESTFN. By using chown with two users, it will be guaranteed the case will be tried.

There is other way though, iterating all users and check whether the user is not the owner of support.TESTFN or not. If you object it, I can use this way.
msg221472 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-24 15:57
Why the need of "as _" in the with statement? I don't understand it. Otherwise, looks good to me.
msg221610 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-06-26 15:58
Okay, I removed "as _". I thought it was not possible.
msg221613 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-26 16:07
Looks good to me.
msg230031 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-26 19:44
Added a couple review comments on possible further improvements.
msg230422 - (view) Author: Cormac O'Brien (cormac-obrien) Date: 2014-11-01 01:57
The unit test has changed significantly since R. David Murray posted his suggestions, but I've at least moved the test over to the new format as noted in the last comment of the v5 patch.
msg230442 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-11-01 09:12
Here is the patch based on R. David Murray's review. Thanks!

Thanks also for the work of Cormac O'Brien.
msg233094 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2014-12-25 09:17
Here is the latest patch which executes unittest.main inside __main__ based on R. David Murray's review. Thanks!
msg233100 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-12-25 23:38
New changeset 32c5b9aeee82 by R David Murray in branch 'default':
#20069: Add tests for os.chown.
https://hg.python.org/cpython/rev/32c5b9aeee82
msg233101 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-26 00:24
Thanks, Vajrasky.
msg236090 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-16 06:32
Tests failed on Solaris:
http://buildbot.python.org/all/builders/AMD64%20Solaris%2011%20%5BSB%5D%203.x/builds/3895/steps/test/logs/stdio

======================================================================
FAIL: test_chown_without_permission (test.test_os.ChownFileTests)
----------------------------------------------------------------------
PermissionError: [Errno 1] Not owner: '@test_18916_tmp'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/cpython/buildslave/cc-32/3.x.snakebite-solaris11-amd64/build/Lib/test/test_os.py", line 1080, in test_chown_without_permission
    os.chown(support.TESTFN, uid_2, gid)
AssertionError: "Operation not permitted" does not match "[Errno 1] Not owner: '@test_18916_tmp'"

----------------------------------------------------------------------
msg236091 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-02-16 06:37
New changeset 4cea99f51bfe by Serhiy Storchaka in branch 'default':
Issue #20069: Fixed test_os on Solaris: error message is platform depended.
https://hg.python.org/cpython/rev/4cea99f51bfe
History
Date User Action Args
2022-04-11 14:57:55adminsetgithub: 64268
2015-02-16 06:37:22python-devsetmessages: + msg236091
2015-02-16 06:32:37serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg236090
2014-12-26 00:24:23r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg233101

stage: commit review -> resolved
2014-12-25 23:38:11python-devsetnosy: + python-dev
messages: + msg233100
2014-12-25 09:17:44vajraskysetfiles: + add_unit_test_os_chown_v7.patch

messages: + msg233094
2014-11-01 09:12:08vajraskysetfiles: + add_unit_test_os_chown_v6.patch

messages: + msg230442
2014-11-01 01:57:52cormac-obriensetfiles: + unittest_os_main.patch
nosy: + cormac-obrien
messages: + msg230422

2014-10-26 19:44:25r.david.murraysetnosy: + r.david.murray
messages: + msg230031
2014-07-03 09:29:04Claudiu.Popasetnosy: + berker.peksag
2014-06-26 16:07:27Claudiu.Popasetmessages: + msg221613
stage: patch review -> commit review
2014-06-26 15:58:06vajraskysetfiles: + add_unit_test_os_chown_v5.patch

messages: + msg221610
2014-06-24 15:57:50Claudiu.Popasetmessages: + msg221472
2014-06-24 15:49:36vajraskysetfiles: + add_unit_test_os_chown_v4.patch

messages: + msg221471
2014-06-20 07:58:17Claudiu.Popasetversions: + Python 3.5, - Python 3.3
2014-06-20 07:55:22Claudiu.Popasetmessages: + msg221072
2014-06-18 15:25:56Claudiu.Popasetmessages: + msg220942
2014-06-18 15:21:40vajraskysetfiles: + add_unit_test_os_chown_v3.patch

messages: + msg220941
2014-06-14 21:35:02Claudiu.Popasetnosy: + Claudiu.Popa
messages: + msg220582
2014-02-23 13:29:02vajraskysetfiles: - add_unit_test_os_chown.patch
2014-02-23 13:28:45vajraskysetfiles: + add_unit_test_os_chown_v2.patch

messages: + msg211993
2014-02-15 15:14:39ezio.melottisetnosy: + ezio.melotti

type: behavior -> enhancement
stage: patch review
2013-12-26 10:28:52vajraskysetfiles: + add_unit_test_os_chown_python27.patch

messages: + msg206937
2013-12-26 10:28:23vajraskycreate