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.

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

test_posix: Android 'id -G' is entirely wrong or missing the effective gid #71131

Closed
xdegaye mannequin opened this issue May 4, 2016 · 15 comments
Closed

test_posix: Android 'id -G' is entirely wrong or missing the effective gid #71131

xdegaye mannequin opened this issue May 4, 2016 · 15 comments
Assignees
Labels
3.7 tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@xdegaye
Copy link
Mannequin

xdegaye mannequin commented May 4, 2016

BPO 26944
Nosy @vstinner, @larryhastings, @bitdancer, @xdegaye, @serhiy-storchaka, @moreati, @yan12125
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • test_getgroups.patch
  • test_getgroups_2.patch
  • test_getgroups_3.patch
  • test_getgroups_4.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/xdegaye'
    closed_at = <Date 2016-10-19.09:11:29.617>
    created_at = <Date 2016-05-04.08:01:13.201>
    labels = ['3.7', 'type-bug', 'tests']
    title = "test_posix: Android 'id -G' is entirely wrong or missing the effective gid"
    updated_at = <Date 2017-03-31.16:36:11.270>
    user = 'https://github.com/xdegaye'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:11.270>
    actor = 'dstufft'
    assignee = 'xdegaye'
    closed = True
    closed_date = <Date 2016-10-19.09:11:29.617>
    closer = 'xdegaye'
    components = ['Tests']
    creation = <Date 2016-05-04.08:01:13.201>
    creator = 'xdegaye'
    dependencies = []
    files = ['42943', '43817', '44143', '45067']
    hgrepos = []
    issue_num = 26944
    keywords = ['patch']
    message_count = 15.0
    messages = ['264789', '264793', '265251', '266070', '270936', '270937', '270943', '271257', '271273', '271280', '271285', '271291', '273068', '278531', '278969']
    nosy_count = 8.0
    nosy_names = ['vstinner', 'larry', 'r.david.murray', 'xdegaye', 'python-dev', 'serhiy.storchaka', 'Alex.Willmer', 'yan12125']
    pr_nums = ['552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26944'
    versions = ['Python 3.6', 'Python 3.7']

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 4, 2016

    test_posix fails on an android emulator running an x86 system image at API level 21.

    On android we have instead of a list of group IDs:
    root@generic_x86:/data/local/tmp # id -G
    uid=0(root) gid=0(root) groups=1003(graphics),1004(input),1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats) context=u:r:su:s0

    ======================================================================
    ERROR: test_getgroups (test.test_posix.PosixTester)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/sdcard/org.bitbucket.pyona/lib/python3.6/test/test_posix.py", line 815, in test_getgroups
        set([int(x) for x in groups.split()]),
      File "/sdcard/org.bitbucket.pyona/lib/python3.6/test/test_posix.py", line 815, in <listcomp>
        set([int(x) for x in groups.split()]),
    ValueError: invalid literal for int() with base 10: 'uid=0(root)'

    Ran 83 tests in 0.114s

    FAILED (errors=1, skipped=11)
    test test_posix failed
    1 test failed:
    test_posix
    Total duration: 0:00:01

    @xdegaye xdegaye mannequin added stdlib Python modules in the Lib dir build The build process and cross-build labels May 4, 2016
    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented May 4, 2016

    See more information in comments in bpo-26932. Since the id tool looks fixed in Android 6.x we should skip the test on Android <6.x.

    @serhiy-storchaka serhiy-storchaka added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error and removed stdlib Python modules in the Lib dir build The build process and cross-build labels May 4, 2016
    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 10, 2016

    'id -G' produces the expected result on an android-23-x86 emulator running Android 6.0 (API 23).

    There is a new error now instead of the one reported at issue bpo-26932:

    ======================================================================
    FAIL: test_getgroups (test.test_posix.PosixTester)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/sdcard/org.bitbucket.pyona/lib/python3.6/test/test_posix.py", line 816, in test_getgroups
        set(posix.getgroups() + [posix.getegid()]))
    AssertionError: Items in the second set but not the first:
    0

    The problem is that posix.getegid() returns 0, and 0 is not in the list printed by 'id -G'.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented May 22, 2016

    test_getgroups is ok on an android emulator running API 23 with the attached patch, and the test is skipped when running API 21.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Jul 21, 2016

    New patch with cosmetic changes.

    @xdegaye xdegaye mannequin self-assigned this Jul 21, 2016
    @larryhastings
    Copy link
    Contributor

    larryhastings commented Jul 21, 2016

    Is there a plan to make Android a supported platform in 3.6?

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Jul 21, 2016

    Is there a plan to make Android a supported platform in 3.6?

    Answer in the Android meta-issue at msg 270942.

    @vstinner
    Copy link
    Member

    vstinner commented Jul 25, 2016

    ValueError: invalid literal for int() with base 10: 'uid=0(root)'

    Hum, does the id program supports -G on Android? The output looks like the regular id output (without -G). Example on my Linux (Fedora 24):

    $ id
    uid=1000(haypo) gid=1000(haypo) groupes=1000(haypo),10(wheel) contexte=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
    
    $ id -G
    1000 10

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Jul 25, 2016

    Hum, does the id program supports -G on Android?

    It does on Android 6.0 but prints the same result as 'id' (without -G) on previous Android versions.

    Here is the output of the commands involved in the test for the root user on my archlinux box, the Android 5.0 emulator and the Android 6.0 emulator:

    archlinux:
      [root@bilboquet default]# python
      ...
      >>> from posix import getegid, getgroups
      >>> getgroups()
      [0, 1, 2, 3, 4, 6, 10, 19]
      >>> getegid()
      0
      [root@bilboquet default]# id -G
      0 1 2 3 4 6 10 19
    
    Android 5.0 (API level 21)
      root@generic_x86:/data/data/org.bitbucket.pyona # python
      ...
      >>> from posix import getegid, getgroups
      >>> getgroups()
      [1003, 1004, 1007, 1011, 1015, 1028, 3001, 3002, 3003, 3006]
      >>> getegid()
      0
      root@generic_x86:/data/data/org.bitbucket.pyona # id -G
      uid=0(root) gid=0(root) groups=1003(graphics),1004(input),1007(log),1011(adb),1015(sdcard_rw),1028(sdcard_r),3001(net_bt_admin),3002(net_bt),3003(inet),3006(net_bw_stats)
    
    Android 6.0 (API level 23)
      root@generic_x86:/data/data/org.bitbucket.pyona # python
      ...
      >>> from posix import getegid, getgroups
      >>> getgroups()
      [1004, 1007, 1011, 1015, 1028, 3001, 3002, 3003, 3006]
      >>> getegid()
      0
      root@generic_x86:/data/data/org.bitbucket.pyona # id -G
      1004 1007 1011 1015 1028 3001 3002 3003 3006

    @vstinner
    Copy link
    Member

    vstinner commented Jul 25, 2016

    Sorry but I don't see the point of trying to "fix" the unit test on
    Android if "id -G" doesn't work as expected.

    In fact, I don't really understand the point of such functional test.
    Are we still testing Python? Or are we testing the OS itself?

    I suggest to remove the whole unit test, or skip the whole unit test
    on Android (@skipIf(is_android(), "-G option of id -G is ignored"),
    something like that).

    @vstinner
    Copy link
    Member

    vstinner commented Jul 25, 2016

    Ooops, I missed the part where you say that it works on API level >= 23. So I suggest to only skip the test on Android API level < 23.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Jul 25, 2016

    The test is already skipped by the patch, for Android API level < 23, with the statement:
    raise unittest.SkipTest("need working 'id -G'")

    Do you mean that the test should be skipped instead more explicitly with something like:
    skiIf(support.android_api_level < 23, "-G option of id -G is ignored by Android")

    @bitdancer
    Copy link
    Member

    bitdancer commented Aug 19, 2016

    Haypo: we are testing that our function wrapper (getgroups) returns something that we can parse as matching the results from the 'id' command. This gives us a much more meaningful test than just testing that getgroups returns *something*. That is, we are testing that it returns integers, and that they "look right".

    In that light, I propose an alternate patch.

    @xdegaye
    Copy link
    Mannequin Author

    xdegaye mannequin commented Oct 12, 2016

    Thanks for the patch David, see my comment in Rietveld.
    New patch based on David patch, just a bit simpler.

    @xdegaye xdegaye mannequin added the 3.7 label Oct 12, 2016
    @xdegaye xdegaye mannequin changed the title android: test_posix fails test_posix: Android 'id -G' is entirely wrong or missing the effective gid Oct 19, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 19, 2016

    New changeset fb3e65aff225 by Xavier de Gaye in branch '3.6':
    Issue bpo-26944: Fix test_posix for Android where 'id -G' is entirely wrong
    https://hg.python.org/cpython/rev/fb3e65aff225

    New changeset 465c09937e85 by Xavier de Gaye in branch 'default':
    Issue bpo-26944: Merge with 3.6.
    https://hg.python.org/cpython/rev/465c09937e85

    @xdegaye xdegaye mannequin closed this as completed Oct 19, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants