KaizenTask1077_Unit_test_dassert_timestamp_lt()

jayati1397 commented 2 months ago

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](https://github.com/kaizen-ai/kaizenflow/blob/master/docs/coding/all.write_unit_tests.how_to_guide.md#do-not-use-hdbgdassert).

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

samarth9008 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

samarth9008 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

kaizen-ai / kaizenflow

KaizenTask1077_Unit_test_dassert_timestamp_lt() #1093