Skip to content

Problems in the timeout utility #20

Open
@Blind4Basics

Description

@Blind4Basics

ok, there are actually several problems about it.

Context

So far, it handles properly:

  • error raised in the function of the user while running
  • wrong output from the user
  • execution time too long
  • max buffer limit error

But it has two problems in its current state:

  1. tests are passing if the process just crashes because it runs out of memory
  2. The "nesting status" (describe/it) of the assertions done inside the timeout utility is ambiguous for one, and even worse, may be different depending on what the function passed by the user to the decorator is doing.

Developping point 1:

Used that way:

# differnet behaviours for the user function

def bufferLimitError():
    while 1: print('qkjdhgkjqhfgdg')
def slowAF():
    while 1: pass
def raiseError():  raise KeyError()
def testFails():   test.fail("")
def testPass():    test.pass_()
def floodRAM():    elemental_forms(random_evil_string(15000))    # this explodes the heap

BEHAVIOURS = (slowAF, raiseError, testFails, testPass, floodRAM, bufferLimitError)

@test.describe(f'timeout inside it blocks')
def tests():
    for i,behave in enumerate(BEHAVIOURS):
        @test.it(behave.__name__)
        def _():
            test.timeout(.1 if i!=4 else 8)(behave)

leads to floodRAM displaying no assertion result at all, meaning this is a "win" for the user if all other tests are passing.

image


Developping point 2:

Current state of the decorator:

def timeout(sec):
    def wrapper(func):
        from multiprocessing import Process
        msg = 'Should not throw any exceptions inside timeout'

        def wrapped():
            expect_no_error(msg, func)
            """ >>> func is supposed to do some assertions on the result of the user's function <<< """

        process = Process(target=wrapped)
        process.start()
        process.join(sec)
        if process.is_alive():
            fail('Exceeded time limit of {:.3f} seconds'.format(sec))
            """ >>> This is the problematic assertion <<< """
            process.terminate()
            process.join()
    return wrapper

So, imagining the following possible uses:

@test.describe(f'developping point 2')
def tests():
    @test.it('inside it block')
    def _():
        @test.timeout(1)
        def _():
            test.expect(True)
            
    @test.describe('wrapping describe/it blocks')
    def _():
        
        @test.timeout(1)
        def _():
            @test.it('lvl 1')
            def _():
                test.expect(True)
                
        @test.timeout(1)
        def _():
            @test.describe('meh')
            def _():
                @test.it('lvl 2')
                def _():
                    test.expect(True)

This leads to assertions that can be done outside of a @test.it block, which is what we are currently fighting against...
For now, the documentation tells nothing about that (about "how to use or not the timeout decorator", I mean)

image


Suggestions

Resolving point 1

I already found a way to handle this. I still have to test the behaviour in different situations to check that it works as expected.
The idea is to pass a Value to the Process, so that this value is updated only if the decorated function doesn't raise/crash. This value is examined afterward at the end of the wrapper to check that the process actually updated it

Resolving point 2

Here I'd need some guidance... I see mostly 3 ways out of this:

  1. State in the documentation that the timeout utility has to be used only inside a @test.it block. This way, nothing to change in the code about that.
  2. State in the documentation that the timeout utility has to be used wrapping the test.it block(s). Then we can add a new it block at the end of the wrapper to get the correct level.
  3. the flexible way... but might lead to more troubles than anything: add a new argument to the decorator stating if the final assertions of the wrapper must be made insde a it block or not. I sort of like the idea, and dislike it at the same time... :/

What would you prefere?

Corollary:

I blieve that the tests checking the framework (python-test-framework/tests/fixtures/) need an update too, to test for all those scenarii (except maybe the buffer limit error?)

final question?

That may be the occasion to add the option asked by Voile (see #4). Even if it' effectively doesn't seem that useful (see FArekkusu's answer), I like the idea of giiving more flexibility to the user.
Go or no go?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions