Skip to content

Added library tests - #88

Merged
vineetbansal merged 3 commits into
mainfrom
add-library-tests
Mar 30, 2026
Merged

Added library tests#88
vineetbansal merged 3 commits into
mainfrom
add-library-tests

Conversation

@owenyu23

Copy link
Copy Markdown
Collaborator

Added test cases for library.py covering:

Oligo construction (forward, reverse, and library oligos)
Basic output structure
Pool splitting across multiple pools
Unix line endings (xfail due to known bug for previous line splitting, which I addressed in fix-library-todos branch)

Comment thread tests/test_library.py Outdated

def mock_library_side_effect(_organism, genes, _n_guides):
# Helper mock to simulate returning one gRNA per gene
return {gene: [{"grna": "ACGTACGTACGTACGTACGT"}] for gene in genes}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you save this grna as a module level variable here, and when testing against it in any test, compare against the variable instead of its value?

Comment thread tests/test_library.py Outdated
grna = result["results"][0]["library"][0][0]

# Verify forward oligo = 5' overhang + gRNA sequence
assert grna["forward_oligo"] == "CACCACGTACGTACGTACGTACGT"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For the forward/reverse oligos comparison, can you reach into the library module and pull out the oligos in the test instead of assuming they are certain values?

Comment thread tests/test_library.py
assert grna["reverse_oligo"] == "ACGTACGTACGTACGTACGTCAAA"

# Verify full library oligo = F1 barcode + 5' adapter + gRNA + 3' adapter + R1 barcode
assert (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same comments here as the comparisons before this - let's compare against the barcodes found in the code instead of assuming they're always going to remain the same.

Comment thread tests/test_library.py Outdated
@patch("guidescanpy.flask.blueprints.library.get_control_guides")
@patch("guidescanpy.flask.blueprints.library.get_essential_genes")
@patch("guidescanpy.flask.blueprints.library.get_library_info_by_gene")
def test_library_basic_structure(mock_library_info, mock_essential, mock_controls):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since the next test is called "..two_pools..", let's call this test_library_single_pool_structure.

Comment thread tests/test_library.py Outdated
@patch("guidescanpy.flask.blueprints.library.get_control_guides")
@patch("guidescanpy.flask.blueprints.library.get_essential_genes")
@patch("guidescanpy.flask.blueprints.library.get_library_info_by_gene")
def test_library_two_pools(mock_library_info, mock_essential, mock_controls):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the test above for the structure is sufficient and this one is not adding much value. Let's get rid of it.

Comment thread tests/test_parsing.py Outdated


def test_parse_unrecognized_extension(data_folder):
file = os.path.join(data_folder, "test_invalid.csv")

@vineetbansal vineetbansal Mar 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For this test, since the contents of the .csv don't matter, let's use the tmp_path fixture in pytest to create a temporary file and pass it in (let's call it sacCer3_regions.csv to indicate that the file might be valid - it's only the extension we're testing). The file test_invalid.csv can be deleted.

Comment thread tests/test_library.py Outdated
@patch("guidescanpy.flask.blueprints.library.get_control_guides")
@patch("guidescanpy.flask.blueprints.library.get_essential_genes")
@patch("guidescanpy.flask.blueprints.library.get_library_info_by_gene")
def test_library_unix_line_endings(mock_library_info, mock_essential, mock_controls):

@vineetbansal vineetbansal Mar 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test will not xfail since I merged your other PR. However, instead or removing the xfail, let's get rid of this test - the test test_parsing.py::test_parse_txt covers something close to this anyway . If you want, you can replace one of the \n in test_parse_txt with \r\n so we're covering both cases.

@vineetbansal

Copy link
Copy Markdown
Member

Thanks @owenyu23 - see my comments, all mostly minor changes requested.

@vineetbansal
vineetbansal merged commit cb25dc2 into main Mar 30, 2026
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants