search

krzema12 / kotlin-python

Python target for the Kotlin Programming Language. See https://github.com/krzema12/kotlin-python/tree/python-backend/python
https://discuss.kotlinlang.org/t/idea-python-backend/19852
48 stars 1 forks source link

Include compilation and running time in box tests exceptions #23

Closed krzema12 closed 3 years ago

krzema12 commented 3 years ago

This change modifies the exceptions thrown in case of failed tests, so that they contain valuable data like Kotlin compilation time, Python running time, ability to infer if compilation or execution failed and why. Unfortunately it seems impossible with JUnit to include a standard error per test case, it's only included on test class level. That's why I need to attach this information to the exception which in turn is attached to individual test case.

It also tries to handle misbehaving Python processes gracefully (kills the one that run >10 s). It was needed to provide their running time.

Testing

Ran it on a small subset of tests - it now provides some helpful data in python/box.tests/build/test-results/pythonTest that can be further analyzed by some tool (not in scope of this PR). However after this PR and even without any sophisticated tooling, it will be possible to grep through the XML test reports and e.g. count the number of Python running time: still running cases.

By the way, it makes running the whole test suite faster again (~1-2 h instead of 4-5) :tada: Probably thanks to killing Python processes that experience an infinite loop/stack overflow.

Examples

Kotlin compilation issue:

Kotlin compilation time: 1653 ms
org.jetbrains.kotlin.python.test.KotlinCompilationException: Kotlin compilation time: 1653 ms
    at org.jetbrains.kotlin.python.test.BasicIrBoxTest.translateFiles(BasicIrBoxTest.kt:173)
...
Caused by: kotlin.NotImplementedError: An operation is not implemented: IrSpreadElementImpl is not supported yet here
    at org.jetbrains.kotlin.ir.backend.py.utils.MiscKt.TODO(misc.kt:25)
    at org.jetbrains.kotlin.ir.backend.py.transformers.irToPy.BaseIrElementToPyNodeTransformer$DefaultImpls.visitElement(BaseIrElementToPyNodeTransformer.kt:14

Python execution issue:

Python running time: 264 ms
...
  File "/home/piotr/repos/kotlin-python/python/py.translator/testData/out/codegen/irBox/argumentOrder/kt9277_v5.py5089678420921489858/compiled_module.py", line 4790, in Unit_getInstance
    if Unit_instance == None:
NameError: name 'Unit_instance' is not defined
    at org.jetbrains.kotlin.python.test.PythonTestChecker.runPython(JsTestChecker.kt:116

Python running timeout:

Python running time: 10000 ms
...
RecursionError: maximum recursion depth exceeded
    at org.jetbrains.kotlin.python.test.PythonTestChecker.runPython(JsTestChecker.kt:116
krzema12 commented 3 years ago

@SerVB

Just wondering. You mentioned a week ago on Slack that it will be a new generated file. Have you declined that idea or will it be implemented in the future too?

In the future. See "Testing" in PR description :)

...it now provides some helpful data in python/box.tests/build/test-results/pythonTest that can be further analyzed by some tool (not in scope of this PR).

So in this PR I focus on producing needed data from the tests, and I'll work on the the new generated file next time.

SerVB commented 3 years ago

By the way, I see before this PR, I have the following processes running (except the first and the last one, which are run by me) and consuming about 6-8 cores fully after running the box tests:

image

I've looked into them and see that there are endless loops. So I guess that proves your theory: on machines with the low number of cores, these loops reduce the performance of other tests severely 🚀

Before this PR, I have to kill those processes manually. With this PR, no processes are left. Cool!