[GSK-1711] Added check of push output #1394
Conversation
python-client/tests/test_push.py
Outdated
| def test_test_function_and_output(enron_model, enron_data): | ||
|
|
||
| # Define a list of push types and corresponding functions | ||
| push_types = [ |
There was a problem hiding this comment.
Since you're modifying this test anyway, let's take a chance to improve it:
instead of storing push_types list you can turn it into a parameterized test. This will:
- improve parallelization as each test will be able to executed separately
- debuggability: it'll be possible to execute a test for a given set of inputs and know which combination lead to an error
python-client/tests/test_push.py
Outdated
| outputs = {push_type: [] for push_type, _ in push_types} | ||
|
|
||
| # Loop through the range and create push outputs | ||
| for i in range(50): |
There was a problem hiding this comment.
I'm not a big fan of blindly taking 50 first rows of enron. In this case we're not really sure what we're testing on. If we know which input leads to which push generation we should use it explicitly.
For example you can take row #5 that is known to generate 2 pushes and assert that both of them are generated and contain expected results.
python-client/tests/test_push.py
Outdated
| # Check if push output has the correct type | ||
| # Check if the tests created by push are instance of GiskardTest | ||
| for push_type, push_outputs in outputs.items(): | ||
| if push_outputs: |
There was a problem hiding this comment.
related to the comment above, if we know that a specific row should result in a specific set of pushes we shouldn't use if push_outputs, but rather assert len(push_outputs)>0
Hartorn
force-pushed
the
GSK-1710
branch
5 times, most recently
from
ecc9346 to
8cd8e4a
Compare
rabah-khalek
force-pushed
the
GSK-1710
branch
from
8cd8e4a to
a0d659e
Compare
There was a problem hiding this comment.
I'm happy with your implementations @Hartorn! it's mergeable to me.
|
Kudos, SonarCloud Quality Gate passed!
|
additional test for push