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

Retire Python UAT suite #3009

Closed
tolbrino opened this issue Nov 14, 2019 · 9 comments · Fixed by #3045
Closed

Retire Python UAT suite #3009

tolbrino opened this issue Nov 14, 2019 · 9 comments · Fixed by #3045
Labels
area/tests Issues or PRs related to tests kind/improvement
Milestone

Comments

@tolbrino
Copy link
Contributor

tolbrino commented Nov 14, 2019

Currently python2.7 is used, which will be EOL in early 2020. Thus, we should update to the latest LTS version of python.

This requires changes in the CI configuration as well and must be tested on all platforms.

The Python UAT suite shall be retired. The tests need to be inspected for cases which are not handled by the systems tests yet. Such cases shall be ported to the system tests.

Some tests covered by the Python UAT shall be handled outside of the new UAT, see #3065 and aeternity/aeminer#17

@tolbrino tolbrino added area/tests Issues or PRs related to tests kind/improvement labels Nov 14, 2019
@tolbrino tolbrino added this to To do in Aeternity Core via automation Nov 14, 2019
@ThomasArts
Copy link
Contributor

The phyton tests do contribute very little to the tests we already have in aehttp_suite and some other places. It probably would be worth a day or two to read each test and see whether the same thing is tested elsewhere. Possibly we end up by only a handful of tests that are still worth porting... and probably porting to something else than python.

The python tests have one advantage, viz. that people not knowing Erlang see how the interface works. But, the question is whether this is still an advantage with all the SDKs we have at the moment. Users can look in a python SDK, a Go SDK or any other language of their taste to see how the interface works.

@sennui
Copy link
Contributor

sennui commented Nov 14, 2019

There were two original ideas around python tests.

  1. The first, was to re-use json objects that are used in the test to build documentation with tutorials. Having tests would make them always up to date.

  2. The second, was to have non-erlang implemetation of API usage. It's supposedly easier to read then erlang. Potentially it would also caught bugs with serialization.

I think none of the original reasons is valid now. For api doc we use swagger. For non-erlang approach we have SDKs. I am fine with removing python test framework.

@tolbrino
Copy link
Contributor Author

I agree, the SDKs give a good idea already of how to use the API in the various languages. Porting the python tests to the system test suite makes sense as it reduces the complexity of the test setups.

The python UAT is in a weird state at the moment, since it was intended to become a lot more but @lucafavatella couldn't finish that vision.

@evdoks
Copy link
Contributor

evdoks commented Nov 15, 2019

Any ideas on who start working on this task? Do we have volunteers? Maybe @skkw, once he is done with GC.

@evdoks
Copy link
Contributor

evdoks commented Nov 19, 2019

As discussed, let's follow @ThomasArts proposal by checking if there are tests worth porting and retire python UAT.

@evdoks evdoks added this to the 5.2.0 milestone Nov 19, 2019
@tolbrino tolbrino changed the title Upgrade UAT suite to python3 Retire Python UAT suite Nov 21, 2019
@evdoks evdoks modified the milestones: 5.2.0, 5.3.0 Nov 25, 2019
@ThomasArts
Copy link
Contributor

ThomasArts commented Nov 25, 2019

Some preliminary cover statistics:

Only in "uats": [{{aec_peers,299},unique,8},
                 {{aec_peers,544},unique,1},
                 {{aec_peers,1078},unique,1},
                 {{aec_peers,1081},unique,1},
                 {{aec_peers,1190},unique,1},
                 {{aec_mining,149},unique,200},
                 {{aetx,495},unique,55},
                 {{aestratum_appsup,17},unique,16},
                 {{aemon_app,17},unique,16},
                 {{aemon_app,20},unique,16},
                 {{aec_peers_pool,993},unique,1},
                 {{aec_peers_pool,1146},unique,1},
                 {{aec_peers_pool,1281},unique,10},
                 {{aec_peers_pool,1495},unique,5},
                 {{aec_eper_metrics_probe,77},unique,470},
                 {{aec_eper_metrics_probe,78},unique,470},
                 {{aec_eper_metrics_probe,90},unique,470},
                 {{aec_eper_metrics_probe,93},unique,470},
                 {{aec_eper_metrics_probe,94},unique,470},
                 {{aec_eper_metrics_probe,98},unique,470},
                 {{aec_eper_metrics_probe,121},unique,470},
                 {{aec_eper_metrics_probe,122},unique,470},
                 {{aec_eper_metrics_probe,124},unique,1802},
                 {{aec_eper_metrics_probe,125},unique,1724},
                 {{aec_eper_metrics_probe,127},unique,78},
                 {{aec_eper_metrics_probe,132},unique,858},
                 {{aec_eper_metrics_probe,134},unique,78},
                 {{aec_eper_metrics_probe,136},unique,2106},
                 {{aec_eper_metrics_probe,137},unique,1248},
                 {{aec_eper_metrics_probe,139},unique,858},
                 {{aehttp_api_handler,59},unique,18},
                 {{aehttp_api_handler,65},unique,18},
                 {{aec_hard_forks,136},unique,549},
                 {{aec_hard_forks,138},unique,549},
                 {{aehttp_app,93},unique,16},  %% due to cover measuring
                 {{aehttp_app,94},unique,16}]  %% due to cover measuring

So there is not a whole lot more that is tested in uat tests. Looking into this more carefully now.

@ThomasArts ThomasArts moved this from To do to In progress in Aeternity Core Nov 26, 2019
@ThomasArts
Copy link
Contributor

ThomasArts commented Nov 26, 2019

aehttp_api_handler differences fixed via: 24d7c61
aec_peers and aec_peers_pool difference fixed via 8aab4a3
aetx difference fixed via 90f96b8
aec_miner revealed some dead code fixed via 93aa43a

aemon_app can be ignored, we never stop the aemon application in tests, but this is trivial code.
aestratum_appsup can be ignored. Another stop function not noticably called in the Erlang tests.

aec_hard_forks irrelevant lines, this sets the start height to Lima. This code is only used for testing and possibly also used in system test. Anyway, can be ignored.

We should get some real tests for aec_eper_metrics_probe. Python tests were not at all doing a test of it, just a side effect of running with a node. System test will do the same.

@ThomasArts
Copy link
Contributor

In Python tests we validate Swagger file. Need to verify whether we can do enough validation on Erlang side to mimic that.

@ThomasArts
Copy link
Contributor

In Python tests we validate Swagger file. Need to verify whether we can do enough validation on Erlang side to mimic that.

Added a test to validate swagger as part of Erlang common test cycle.
f354f45

@evdoks evdoks modified the milestones: 5.3.0, 5.4.0 Dec 18, 2019
@evdoks evdoks modified the milestones: 5.4.0, 5.5.0 Jan 14, 2020
Aeternity Core automation moved this from In progress to Done Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests Issues or PRs related to tests kind/improvement
Projects
Development

Successfully merging a pull request may close this issue.

4 participants