Skip to content

Commit

Permalink
Cleanup testing: refactor and standardize. (#237)
Browse files Browse the repository at this point in the history
* Cleanup testing: refactor and standardize.

Rename `fixtures_constants` to `test_support` as it will also contain test utilities that can be used to make tests more readable.

Tests should now follow a similar structure (test file, test model changes, test folder, test deep folder, test empty, test special), with similar test names and similar spacing between the lines (in general arrange-act-assert, except for tests such as `test_model_with_empty_folder_hashes_differently_than_with_empty_file` which need to create an empty dir, compute a digest, delete the dir, create an empty file, compute a new digest, compare the 2 digests). Some tests are not duplicated in the other class as they don't make sense (e.g. itemized serializers produce no result on empty folders). Some tests have slightly different names in the other class due to behavior differences (e.g., corner cases around empty files).

Remove the `test_special_file` that tests for FIFOs in every test suite as we only need to test it once (this way, the special way this test is written is not replicated multiple times).

The code refactoring in the next PR will also remove some of the corner cases around these tests.

Since Critique gets confused by lines changed in the PR (as tests got reordered), it might be better to just review the file in its entirety

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>

* Remove unused imports

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>

* Fix line length

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>

* Simplify tests

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>

---------

Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>
  • Loading branch information
mihaimaruseac authored Jul 16, 2024
1 parent 73578c1 commit 516f6f6
Show file tree
Hide file tree
Showing 5 changed files with 395 additions and 460 deletions.
4 changes: 2 additions & 2 deletions model_signing/serialization/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

import pytest

from model_signing.serialization import fixtures_constants
from model_signing.serialization import test_support


# Note: Don't make fixtures with global scope as we are altering the models!
@pytest.fixture
def sample_model_file(tmp_path_factory):
file = tmp_path_factory.mktemp("model") / "file"
file.write_bytes(fixtures_constants.KNOWN_MODEL_TEXT)
file.write_bytes(test_support.KNOWN_MODEL_TEXT)
return file


Expand Down
18 changes: 0 additions & 18 deletions model_signing/serialization/fixtures_constants.py

This file was deleted.

Loading

0 comments on commit 516f6f6

Please sign in to comment.