mathesar icon indicating copy to clipboard operation
mathesar copied to clipboard

Split casting tests for speed

Open mathemancer opened this issue 2 years ago • 4 comments

Problem

Currently, we have some tests in db/tests/types/operations/test_cast.py that run quite slowly, and are parameterized to run many times. We should split them up. For example, the test alter_column_casts_data_gen is set up to test that we can alter columns correctly, and that data is correctly cast from one type to another. This is inefficient at scale, since it mixes testing that the column is altered and the data is cast. We can test the data casting without actually creating any column (for the different inputs), and only need one per type pair to make sure the alter column function is wired up properly for each.

Proposed solution

We should explore the tests in the above file, and try to split them into multiple, quicker-running tests. We should avoid reusing slow tests to test things which would be done more efficiently otherwise.

Marked as draft until we have a more precise picture of how to change the tests in question.

mathemancer avatar Mar 04 '22 13:03 mathemancer

Thanks for bringing this up. I've been killing pytest mid-run to bypass casting tests, since they take a lot of time.

Tests that test multiple things at once are common in our suites, which is inefficient, as you've pointed out. I've definitely contributed to that. I think more care during PR reviews and general opportunistic refactoring should be welcomed.

For the time being, I'll add --ignore=db/tests/types/operations/test_cast.py to some of my test calls. Maybe I'll make a shell alias for it.

dmos62 avatar Mar 07 '22 12:03 dmos62

Hi,

I know this issue isn't labelled with the "good first issue" tag, but can I give this issue a go? I recently worked with this file and I'd like to understand Mathesar's types system better.

Jayitha avatar May 10 '22 08:05 Jayitha

@Jayitha hey, thanks for volunteering! You're welcome to have at it, but I'm doing a refactor that makes significant changes to this file, so I highly suggest you base off of that. Feel free to reach me on our matrix if anything. My handle is @dominykas:matrix.mathesar.org.

Also, you might be interested in this issue where I outline a few problems with our test suite https://github.com/centerofci/mathesar/issues/1368

dmos62 avatar May 10 '22 12:05 dmos62

@dmos62 I will work off the refactor as specified. Thank you!

Jayitha avatar May 11 '22 00:05 Jayitha

This issue has not been updated in 90 days and is being marked as stale.

github-actions[bot] avatar Feb 04 '23 21:02 github-actions[bot]