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

Allow testing with python3 #1725

Merged
merged 15 commits into from
Feb 18, 2021
Merged

Allow testing with python3 #1725

merged 15 commits into from
Feb 18, 2021

Conversation

g1itch
Copy link
Collaborator

@g1itch g1itch commented Jan 29, 2021

Hello!

I'm trying to implement first step of #1712. Any tests module calling tests.common.skip_python3() upon import will be skipped when ran with python3. Currently all modules call it because of imports: for python3 you probably need another pathmagick.

@g1itch g1itch force-pushed the ci-python3 branch 2 times, most recently from 32a3449 to feb308b Compare January 30, 2021 15:41
@PeterSurda
Copy link
Member

The missing python binaries/headers for travis2bash should be fixed now.

@g1itch g1itch force-pushed the ci-python3 branch 12 times, most recently from 389ae72 to 5e7a9a5 Compare February 7, 2021 17:30
@PeterSurda
Copy link
Member

PeterSurda commented Feb 8, 2021

  • I tried to fix buildbot/dpkg job, I manually install the needed packages with apt, but now it aborts due to inability to find tests
  • why delete test_is_own_address_add_to_addressbook? maybe you want to make it into a UI test?

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 8, 2021

* I tried to fix `buildbot/dpkg` job, I manually install the needed packages with apt, but now it aborts due to inability to find tests

Your buildbot warns about disallowed host and references nonexistent rftd page:
Note: Bypassing https://pypi.python.org/simple/pycrypto/ (disallowed host; see http://bit.ly/2hrImnY for details).

* why delete `test_is_own_address_add_to_addressbook`? maybe you want to make it into a UI test?

I forgot the second part: 828dbd0. Now I'm investigating skipping of qt tests in this branch ):

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 8, 2021

You can check this build to see that addressbook test requires Qt. Marking core tests as passed when it exited after an error is even worse than permanent failures.

@PeterSurda
Copy link
Member

Your buildbot warns about disallowed host and references nonexistent rftd page:
Note: Bypassing https://pypi.python.org/simple/pycrypto/ (disallowed host; see http://bit.ly/2hrImnY for details).

I tried to fix that but I'm not sure what's causing it. I manually added *.python.org to allowed hosts but there was no change. Instead I then changed the preparation steps to install the needed packages through apt instead of waiting for dpkg to use easy_install, but now it's protesting about something else.

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 8, 2021

Instead I then changed the preparation steps to install the needed packages through apt instead of waiting for dpkg to use easy_install, but now it's protesting about something else.

I see, it cannot find tests.py which is now used to run tests because it's not copied into virtualenv (it is not installed like tests package itself).

Copy link
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

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

well done, I'm happy

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 17, 2021

Why do you want to run it twice, is it a bootsrapping workaround?

I want to put to the proof the quality of the tests themself. To avoid the situation when some tests rely on the side effects of other. Bootstrapping is tested in core tests part which I don't duplicate and neither randomize the order. I don't see there a problem with bootstrap tests though.

I can remove the second pass if you don't like it. You asked about reduced number of tests in the suite. Currently there are 48 of them with one pass and 96 with two passes.

I would recommend instead the modify the network code to handle premature disconnects better.

I see this as a parallel track, the different task that should address #1597 and maybe some other existing issues like stale number of the objects to be synced that is observed some times.

Agreed on the rest part.

@@ -151,7 +150,7 @@ def _ec_point_serialize(self, point):
# padding manually
bx = OpenSSL.malloc(0, OpenSSL.BN_num_bytes(x))
OpenSSL.BN_bn2bin(x, bx)
out = bx.raw.rjust(l_, chr(0))
out = bx.raw.rjust(l_, chr(0).encode())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@PeterSurda maybe use literal b'\x00' here?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't looked at this aspect of pyelliptic yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is rather an aspect of python3, where bytes and str are treated separately

@PeterSurda
Copy link
Member

I want to put to the proof the quality of the tests themself. To avoid the situation when some tests rely on the side effects of other. Bootstrapping is tested in core tests part which I don't duplicate and neither randomize the order. I don't see there a problem with bootstrap tests though.

I can remove the second pass if you don't like it. You asked about reduced number of tests in the suite. Currently there are 48 of them with one pass and 96 with two passes.

I think as a temporary workaround it's ok, as long as it doesn't take too much time. I want a single travis job to run under 5 minutes, the less the better. In particular without having to have fast CPU. Perhaps we can profile the tests to see where duration can be reduced?

Over long term we should have the non-deterministic tests fixed properly. I understand however we can't do it immediately due to complicated inter-dependencies, and for test purposes we still need to know what's happening.

I see this as a parallel track, the different task that should address #1597 and maybe some other existing issues like stale number of the objects to be synced that is observed some times.

Ok.


[testenv:stats]
commands =
coverage report
Copy link
Collaborator Author

@g1itch g1itch Feb 17, 2021

Choose a reason for hiding this comment

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

Name                                      Stmts   Miss  Cover
-------------------------------------------------------------
src/addresses.py                            167     69    59%
src/api.py                                  646    473    27%
src/bitmessagemain.py                       289    138    52%
src/bitmessageqt/account.py                 193    193     0%
src/bitmessageqt/address_dialogs.py         188    188     0%
src/bitmessageqt/addressvalidator.py        106    106     0%
src/bitmessageqt/bitmessage_icons_rc.py       9      9     0%
src/bitmessageqt/bitmessageui.py            748    748     0%
src/bitmessageqt/blacklist.py               149    149     0%
src/bitmessageqt/dialogs.py                  42     42     0%
src/bitmessageqt/foldertree.py              381    381     0%
src/bitmessageqt/languagebox.py              27     27     0%
src/bitmessageqt/messagecompose.py           16     16     0%
src/bitmessageqt/messageview.py              99     99     0%
src/bitmessageqt/migrationwizard.py          59     59     0%
src/bitmessageqt/networkstatus.py           118    118     0%
src/bitmessageqt/newchandialog.py            40     40     0%
src/bitmessageqt/retranslateui.py            17     17     0%
src/bitmessageqt/safehtmlparser.py           93     93     0%
src/bitmessageqt/settings.py                303    303     0%
src/bitmessageqt/settingsmixin.py            56     56     0%
src/bitmessageqt/sound.py                    10     10     0%
src/bitmessageqt/statusbar.py                28     28     0%
src/bitmessageqt/support.py                  87     87     0%
src/bitmessageqt/uisignaler.py               62     62     0%
src/bitmessageqt/utils.py                    61     61     0%
src/bitmessageqt/widgets.py                  11     11     0%
src/bmconfigparser.py                       100     22    78%
src/build_osx.py                              9      9     0%
src/class_addressGenerator.py               196    163    17%
src/class_objectProcessor.py                571    503    12%
src/class_singleCleaner.py                   80     53    34%
src/class_singleWorker.py                   590    502    15%
src/class_smtpDeliver.py                     90     90     0%
src/class_smtpServer.py                     143    143     0%
src/class_sqlThread.py                      331    195    41%
src/debug.py                                 37      9    76%
src/defaults.py                               4      0   100%
src/depends.py                              205    110    46%
src/helper_ackPayload.py                     21      8    62%
src/helper_addressbook.py                     6      1    83%
src/helper_bitcoin.py                        37     32    14%
src/helper_inbox.py                          10      5    50%
src/helper_msgcoding.py                     108     23    79%
src/helper_random.py                         21      6    71%
src/helper_search.py                         58     58     0%
src/helper_sent.py                           27      1    96%
src/helper_sql.py                            64     21    67%
src/helper_startup.py                       182     73    60%
src/highlevelcrypto.py                       69     32    54%
src/inventory.py                             20      3    85%
src/l10n.py                                  75     43    43%
src/main.py                                  10     10     0%
src/messagetypes/message.py                  22      2    91%
src/messagetypes/vote.py                     18     11    39%
src/multiqueue.py                            25      4    84%
src/network/addrthread.py                    36     15    58%
src/network/advanceddispatcher.py           116     13    89%
src/network/announcethread.py                26      5    81%
src/network/assemble.py                      21      2    90%
src/network/asyncore_pollchoose.py          648    339    48%
src/network/bmobject.py                      77     44    43%
src/network/bmproto.py                      399    167    58%
src/network/connectionchooser.py             56     15    73%
src/network/connectionpool.py               251     43    83%
src/network/constants.py                      6      0   100%
src/network/dandelion.py                    100     50    50%
src/network/downloadthread.py                63      9    86%
src/network/http.py                          64     64     0%
src/network/httpd.py                         93     93     0%
src/network/https.py                         35     35     0%
src/network/invthread.py                     77     47    39%
src/network/knownnodes.py                   151     26    83%
src/network/networkthread.py                 32     13    59%
src/network/node.py                           3      0   100%
src/network/objectracker.py                  86     39    55%
src/network/proxy.py                         70     19    73%
src/network/receivequeuethread.py            35      9    74%
src/network/socks4a.py                       79     59    25%
src/network/socks5.py                       114     40    65%
src/network/stats.py                         36     19    47%
src/network/tcp.py                          248     59    76%
src/network/threads.py                       27      0   100%
src/network/tls.py                          138     39    72%
src/network/udp.py                          102     48    53%
src/network/uploadthread.py                  48     21    56%
src/openclpow.py                             81     57    30%
src/pathmagic.py                              7      0   100%
src/paths.py                                 75     50    33%
src/plugins/desktop_xdg.py                   17     17     0%
src/plugins/indicator_libmessaging.py        41     41     0%
src/plugins/menu_qrcode.py                   63     63     0%
src/plugins/notification_notify2.py          11     11     0%
src/plugins/plugin.py                        22     16    27%
src/plugins/proxyconfig_stem.py              71     71     0%
src/plugins/sound_canberra.py                10     10     0%
src/plugins/sound_gstreamer.py               10     10     0%
src/plugins/sound_playfile.py                30     30     0%
src/proofofwork.py                          249    209    16%
src/protocol.py                             265    123    54%
src/pyelliptic/arithmetic.py                105     28    73%
src/pyelliptic/cipher.py                     39     29    26%
src/pyelliptic/ecc.py                       293    209    29%
src/pyelliptic/eccblind.py                  219     18    92%
src/pyelliptic/eccblindchain.py              36      1    97%
src/pyelliptic/hash.py                       41     34    17%
src/pyelliptic/openssl.py                   419     82    80%
src/qidenticon.py                           112    112     0%
src/queues.py                                31      1    97%
src/randomtrackingdict.py                    72     11    85%
src/shared.py                               121     66    45%
src/shutdown.py                              49      3    94%
src/singleton.py                              8      0   100%
src/state.py                                 30      0   100%
src/storage/filesystem.py                   152    121    20%
src/storage/sqlite.py                        69     29    58%
src/storage/storage.py                       38     15    61%
src/threads.py                               20      5    75%
src/upnp.py                                 209    209     0%
-------------------------------------------------------------
TOTAL                                     13428   8770    35%

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 17, 2021

I want a single travis job to run under 5 minutes, the less the better.

With one pass one job runs less then 5 min, but one build may run much longer depending on number of jobs included: https://travis-ci.org/github/g1itch/PyBitmessage/builds/759393114

@PeterSurda
Copy link
Member

With one pass one job runs less then 5 min, but one build may run much longer depending on number of jobs included: https://travis-ci.org/github/g1itch/PyBitmessage/builds/759393114

I don't particularly care about travis-ci.org. Even from speed perspective, we'll probably have more and more types of jobs later, and even their most expensive plan only has 5 parallel jobs. I can now run 58 parallel jobs on my buildbot workers, which will be increased to over 100 soon.

@PeterSurda
Copy link
Member

@g1itch how much more work do you need here, when can I merge it, and do you want me to create separate buildbot jobs for tox tests, or replace the existing ones?

@g1itch
Copy link
Collaborator Author

g1itch commented Feb 18, 2021

@g1itch how much more work do you need here, when can I merge it, and do you want me to create separate buildbot jobs for tox tests, or replace the existing ones?

If there are no commits you want me to drop, I'll squash this and merge. I don't think you need to create any buildbot jobs for tox, because it creates extra nested virtualenvs and does not fit to buildbot or travis.

There is an new strange error in api broadcast test: https://buildbot.bitmessage.org/#builders/24/builds/39. I saw it in xenial job at first. And I reproduced it locally with tox a couple of times. I'm afraid it may break one of the required checks in the next build. Maybe I should find the cause. It can be a bug in the test logic or some issue with broadcast msgid.

@PeterSurda
Copy link
Member

PeterSurda commented Feb 18, 2021

TLDR; from my point of view all ok, can be merged.

If there are no commits you want me to drop, I'll squash this and merge.

I reviewed the latest changes and am happy for this to go as it is. Some other developers are working on the android port which requires python3, and they can't merge at the moment, so I'd like the basic support go in as soon as possible. It doesn't need to run yet in python3, just so that the kivy/android tests can be added and run.

I don't think you need to create any buildbot jobs for tox, because it creates extra nested virtualenvs and does not fit to buildbot or travis.

Well, not necessarily, I can control what happens in buildbot. I'll look at it and add it later, after it's merged.

There is an new strange error in api broadcast test: https://buildbot.bitmessage.org/#builders/24/builds/39. I saw it in xenial job at first. And I reproduced it locally with tox a couple of times. I'm afraid it may break one of the required checks in the next build. Maybe I should find the cause. It can be a bug in the test logic or some issue with broadcast msgid.

I'm not bothered with that the moment although it would be nice to have it fixed eventually.

 - use dotted imports, remove unneeded shebangs
 - openssl._OpenSSL._version is of type bytes
 - use b'\x00' literal instead of chr(0) in eccblind and test_openssl
 - use // and divmod in arithmetic to fit PEP238:
   https://docs.python.org/3/whatsnew/2.2.html#pep-238-changing-the-division-operator
 - remove import from debug
 - use divmod and bytes
 - use bytes for python3
 - encode the result of arithmetic.privtopub
 - add test for arithmetic.base10_multiply
 - simplify imports
 - signatures are of type bytes
 - chain kwarg of pyelliptic.ECCBlindChain is bytes
  - make separate tests runner - tests.py; python setup.py test still works
  - tox.ini with coverage config
  - -b: issue warnings about comparing bytearray with unicode
  - export PYTHONWARNINGS=all on stage install
@g1itch g1itch merged commit ef849d2 into Bitmessage:v0.6 Feb 18, 2021
@g1itch
Copy link
Collaborator Author

g1itch commented Feb 18, 2021

I guess, tox may be used as an alternate way to run tests on other Linux distros than Ubuntu. I'm trying to use it on Fedora in my ci branch.

As for coverage... it's a pity, I thought there would be some plugins for buildbot for that purpose to process xml, but now I cannot google anything ):

@g1itch g1itch deleted the ci-python3 branch May 28, 2021 14:54
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.

2 participants