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

Update uu.py and test_uu.py from CPython v3.12.0 #5161

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

kingiler
Copy link
Contributor

@kingiler kingiler commented Feb 9, 2024

Pass more unit tests in test_uu.py

@kingiler kingiler marked this pull request as ready for review February 9, 2024 14:52
@fanninpm
Copy link
Contributor

fanninpm commented Feb 9, 2024

Usually, the commits that have anything to do with RustPython are separated from the commits that contain changes made in CPython by CPython developers.

@youknowone
Copy link
Member

One of the failing test is test.test_importlib.builtin.test_loader.Frozen_LoaderTests.test_module.
No idea how the change is related to the failing test yet.

@youknowone
Copy link
Member

@kingiler is this intended to be closed or accident?

@kingiler
Copy link
Contributor Author

kingiler commented Mar 1, 2024

@youknowone It was an accident. I messed up with the git force push commit before.

@kingiler
Copy link
Contributor Author

kingiler commented Mar 1, 2024

Now I found the potential cause for the issue. Comment out introducing of warnings._deprecated(__name__, remove=(3, 13)) in uu.py would pass the test_importlib test.

@kingiler kingiler reopened this Mar 1, 2024
@kingiler
Copy link
Contributor Author

kingiler commented Mar 1, 2024

@youknowone I have a strange issue that this PR does not sync with my branch at https://github.com/kingiler/RustPython/tree/update_uu, do you know what is happening here?

@youknowone
Copy link
Member

youknowone commented Mar 12, 2024

it happens sometimes when you pushed it during parts of github is out of service. you can edit the last commit by git commit --amend without any actual change to generate a new commit id and push force to update it.

Source_LoaderTests
) = util.test_both(LoaderTests, machinery=machinery)
# TODO: RUSTPYTHON
# (Frozen_LoaderTests,
Copy link
Member

Choose a reason for hiding this comment

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

Do you have any idea which part of this test fails?

Copy link
Member

Choose a reason for hiding this comment

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

We can't simply disable importlib test because it is one of the most important part. We need to investigate what's happening here.

Blues-star and others added 13 commits March 15, 2024 19:59
Co-authored-by: Jeong, YunWon <jeong@youknowone.org>
Signed-off-by: wellweek <xiezitai@outlook.com>
The failing test was unsetting `PYTHONPATH`, but neglecting to unset
`RUSTPYTHONPATH`, which obviously was not significant for the original
CPython test. Including `RUSTPYTHONPATH` in the test fixes it.
There are a substantial number of socket tests that are disabled due to
`bind(): bad family` errors. It seems like RustPython only supports a
small subset of the required connection families, so the failing tests
are broken for the same reasons.
Technically speaking, my system was misconfigured, leading me to disable
the test in the first place.

`test_locale` calls `locale.setlocale(locale.LC_ALL, '')`, which reads
the value of the `LANG` environment variable and uses that to look up
and reset all the locale settings. My system has `LANG=en_US.UTF-8`,
which is apparently not what this test was expecting. If `LANG` is unset
or set to `C`, the test passes, as it does in CI.
I had previously `test_locale` as expected to fail, as it did indeed
fail on my system due to unimplemented functionality. As it happens, it
passes in CI because the locale settings used there (`C`, I believe)
just happen to format integers the same with "%d" as "%n".

I mistakenly un-marked it because I thought I misunderstood the problem.
Eclips4 and others added 5 commits March 15, 2024 19:59
…#5201)

After you suggestion in python/cpython#116504 (comment) I went to take a look at `test_cmd_line` in RustPython (it was so long ago I contributed to this amazing project, so may thing had changed!), and I've noticed this.

This is a problem, here' the simplest demo:

```python
import unittest

class TestMe(unittest.TestCase):
    @unittest.expectedFailure
    def test_me(self):
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

This works as expected:

```
» ./python.exe ex.py
x
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK (expected failures=1)
```

This does not:

```python
import unittest

class TestMe(unittest.TestCase):
    def test_me(self):
        @unittest.expectedFailure
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

Produces:

```
» ./python.exe ex.py
E
======================================================================
ERROR: test_me (__main__.TestMe.test_me) (run=<function TestMe.test_me.<locals>.run at 0x1057a2150>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 10, in test_me
    run()
    ~~~^^
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 7, in run
    raise ValueError
ValueError

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)
```

So, I propose to remove these decorators, let's only keep `TODO` comments to indicate separate failures.
@youknowone
Copy link
Member

If removing deprecated warning fix importlib problem, it will be the best.
uu will be removed in 3.13. Probably not worth to spend too much effort though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants