Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Cgi/verifier/scripts for addional tests #4584

Merged
merged 76 commits into from
Nov 5, 2019

Conversation

nieznanysprawiciel
Copy link
Contributor

@nieznanysprawiciel nieznanysprawiciel commented Aug 5, 2019

This PR introduces following changes:

  1. Scripts for extended tests of blender verifier
  2. Semi integrations tests utilities
  3. Tests reproducing some known bugs in blender verifier

Detailed description:

  1. Scripts are checking different subtasks splits on predefined checker scene

    • Tests check if verifier computes crops coordinates correctly and if crops are correctly matched with verified image region.
    • Tests don't use verifier verdict, but instead compare produced crops pixel by pixel with reference crops. This bypasses problem with not ideal verification quality.
    • Scripts execution lasts very long, so they are not included in standard test set. They will be ran once and all discovered bugs will be reproduced using semi-integrations tests. This means that scripts don't affect Golem in any way.
  2. Semi integrations tests utilities:

    • Test utilities help write tests running full application flow (I mean golem integrations from app directory).
    • Utils use TaskManager class to emulate golem behavior as close as possible.
    • Tests don't need network and external nodes. Providers are mocked on local computer.
    • keep_testdir_on_fail decorator was added to TempDirFixture, to easier check content of test temporary directories, when debugging.
    • These utils are used by transcoding branches as well, so merging them will help us to easier merge to develop in the future.
  3. Tests reproducing some known bugs in blender verifier

    • Test reproducing bugs found so far by verifier scripts.
    • Reproduced bugs are already fixed on develop, this PR adds only reproduction that uses semi-integration tests utils.

golem/testutils_app_integration.py Outdated Show resolved Hide resolved
golem/testutils_app_integration.py Outdated Show resolved Hide resolved
golem/testutils_app_integration.py Outdated Show resolved Hide resolved
golem/verifier/blender_verifier.py Outdated Show resolved Hide resolved
tests/apps/blender/task/test_blenderintegration.py Outdated Show resolved Hide resolved
tests/golem/verifier/blenderverifier_ext_test_script.py Outdated Show resolved Hide resolved
tests/golem/verifier/blenderverifier_ext_test_script.py Outdated Show resolved Hide resolved
tests/golem/verifier/blenderverifier_ext_test_script.py Outdated Show resolved Hide resolved
tests/golem/verifier/blenderverifier_ext_test_script.py Outdated Show resolved Hide resolved
# Use this decorator if you don't want to remove test directory
# after test execution.
# This is debug decorator - don't remove even if it isn't used.
def keep_testdir_on_fail(fun):
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't it be better to have an optional command line argument instead of a decorator for this purpose instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefere this solution, because cmdline switch would enable it for all test cases. They produce a lot of garbage and is is a lot harder to find results of only one specific test

self.assertFalse(timeouted)
return self.task_manager.verify_subtask(subtask_id)

def _execute_subtask(self, task: Task, ctd: dict):
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason some of those methods here are public and some are private? are any of them called from outside the test class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of them are designed to be called outside of TestTaskIntegration and some of them are not. But not all of them are used yet.

@bidzyyys bidzyyys force-pushed the CGI/verifier/scripts-for-addional-tests branch from cadbd19 to d950a7f Compare October 31, 2019 16:28
@bidzyyys bidzyyys requested a review from etam November 2, 2019 10:52
Copy link
Contributor

@etam etam left a comment

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@shadeofblue shadeofblue left a comment

Choose a reason for hiding this comment

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

mckayla okayish

@magdasta magdasta merged commit 55ba1ef into develop Nov 5, 2019
@bidzyyys bidzyyys deleted the CGI/verifier/scripts-for-addional-tests branch November 5, 2019 14:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants