KaizenTask1077_Unit_test_dassert_timestamp_lt()

[jayati1397]

Created this PR for Issue #1077.

Changes Made:

• Implemented unit tests for dassert_timestamp_lt.

Testing:
Ran code through pylint and pytest.

• Pylint Errors: Class name: currently follows the convention specified here.

OUTPUT from the added lines in the file
helpers/test/test_hdatetime.py:781:0: C0115: Missing class docstring (missing-class-docstring)
helpers/test/test_hdatetime.py:781:0: C0103: Class name "Test_dassert_timestamp_lt" doesn't conform to PascalCase naming style (invalid-name)

• Pytest: No errors.

OUTPUT:
collected 3 items                                                                                                                                       

helpers/test/test_hdatetime.py::Test_dassert_timestamp_lt::test1 (0.00 s) PASSED    [ 33%]
helpers/test/test_hdatetime.py::Test_dassert_timestamp_lt::test2 (0.00 s) PASSED    [ 66%]
helpers/test/test_hdatetime.py::Test_dassert_timestamp_lt::test3 (0.00 s) PASSED    [100%]

Questions:

• Should I test dassert_is_valid_timestamp as part of this issue, or should I create a mock of that function while testing dassert_timestamp_lt?
• Should I write test cases for None values?
• I haven't used dassert_lt as mentioned here.

@samarth9008 please let me know if you have any further suggestions

[samarth9008]

Should I test dassert_is_valid_timestamp as part of this issue, or should I create a mock of that function while testing dassert_timestamp_lt?

No need as it must have been tested separately.

Should I write test cases for None values?

Yes

I haven't used dassert_lt as mentioned here.

Could you clarify your question better?

Will review when above quesitons are resolved.

[jayati1397]

@samarth9008

Could you clarify your question better?

I haven't used dassert_lt as mentioned here. So is this the right way? Or do we use the dassert method

I have added tests for none values

[samarth9008]

I haven't used dassert_lt as mentioned here. So is this the right way? Or do we use the dassert method

Where do you want to use dassert_lt?

[jayati1397]

Where do you want to use dassert_lt?

I can by using mock but would imply breaking the rule of making the unit tests singular(testing on function at a time) and simple

[jayati1397]

Also @samarth9008 can you give me another issue by the time you review this?

[samarth9008]

I can by using mock but would imply breaking the rule of making the unit tests singular(testing on function at a time) and simple

No need to mock as we only use it for external calls

Regarding dassert_lt, I think you are confusing with its usage

• We use hdsg.dassert_* functions to check the invariants within the code. This should hold true other it should raise the assertion
• On the other hand, we use self.assert_* function inside tests to check if the function is generating the output it should to test the functions.

To test hdbg.dassert_lt(start_timestamp, end_timestamp) this line of code, you can supply non-none end timestamp greater than start timestamp and the function should raise an assertion

make sense?

[jayati1397]

I can by using mock but would imply breaking the rule of making the unit tests singular(testing on function at a time) and simple

No need to mock as we only use it for external calls

Regarding dassert_lt, I think you are confusing with its usage

* We use hdsg.dassert_* functions to check the invariants within the code. This should hold true other it should raise the assertion

* On the other hand, we use self.assert_* function inside tests to check if the function is generating the output it should to test the functions.

To test hdbg.dassert_lt(start_timestamp, end_timestamp) this line of code, you can supply non-none end timestamp greater than start timestamp and the function should raise an assertion

make sense?

It makes sense
So in this case we would be testing hdbg.dassert_lt(start_timestamp, end_timestamp)
Do you want me to use that while I am testing dassert_timestamp_lt()

[samarth9008]

So in this case we would be testing hdbg.dassert_lt(start_timestamp, end_timestamp)
Do you want me to use that while I am testing dassert_timestamp_lt()

Yes would be nice to increase the coverage.

[jayati1397]

So in this case we would be testing hdbg.dassert_lt(start_timestamp, end_timestamp)
Do you want me to use that while I am testing dassert_timestamp_lt()

Yes would be nice to increase the coverage.

Also, would you mind reviewing rest of the PR too?

[jayati1397]

So in this case we would be testing hdbg.dassert_lt(start_timestamp, end_timestamp)
Do you want me to use that while I am testing dassert_timestamp_lt()

Yes would be nice to increase the coverage.

I looked into the code and I had confusion how can we test both dassert_lt and dassert_timestamp_lt at the same time
anyways dassert_timestamp_lt is using dassert_lt and dassert_lt is itself raising assertion.

For coverage we can write unti test for dassert_lt(that too not necessary as I understand the code)
Can you help me here in understanding @samarth9008

[jayati1397]

As discussed with @samarth9008 , creating #1094 and #1095 for testing dassert_is_valid_timestamp and dassert_lt respectively

[samarth9008]

Impressive first PR. GJ following all the process.

Few nits from my side.

[samarth9008]

1 nit and LG