-
Notifications
You must be signed in to change notification settings - Fork 20
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
Use async io where possible to improve runtime performance #163
base: main
Are you sure you want to change the base?
Conversation
@netomi thanks! please check the errors in the CI and also do not forget to regen the tests just in case dependencies in tests have been updated https://github.com/nexB/python-inspector#testing |
There are a couple of things that I still need to fix, but the failing CI runs are unrelated to this PR afaict. They also exist for other PRs and look like that the setup is broken. |
ok so there are some unit test failure that I need to address. Most of them are from the fact that python-inspector returned duplicate package entries in its result up to now, which I fixed, so the expected test results need to be adjusted to take this change into account. |
I could fix most tests (weird thing is that 4 tests fail locally when run in pycharm, but work on the command line so that might be a pycharm problem), there are some weird test failures on macos and windows only. Do you have any idea about that? |
still need to take a look at etc/scripts if the there also need to be adjustments after the changes. |
@netomi BTW, do not bother with the etc/scripts ... these come from the shared skeleton repo at https://github.com/nexB/skeleton and should not be of concern for you. |
Can this be moved forward, please? 😬 |
@sschuberth tests are falling, tests needs to be regen |
Would you have time to do this @netomi? |
hmm I can take a look, but I am quite discouraged from contributing to a repo that has to constantly rebuilding its tests and they are failing most of the time when creating a PR. You still need to inspect if its related to your changes or the test data is just out of date. |
@netomi I agree this is not great. What do you think we could do as an alternative? |
please dont actually call live servers during tests but rather mock their responses with data you might have retrieved from the server at one point in time. I understand that is a bit harder to do, but ultimately results in much more stable and reliable tests imho. PyGitHub has a nice system for their tests where they replay data: |
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@gmail.com>
At least one of the errors seems to be
|
no idea where this error is coming from. This decorator is not used anywhere in the code. Probably from a dependency? This error btw also occurs for the default branch, so its not related to this PR afaict. |
@TG1999 or @chinyeungli, can you assist here by fixing the test baseline in |
This PR fixes #162 .
Things that this PR will modify: