[Feature]: Improve zstash testing

[forsyth2]

How will this affect the next version number?

New feature (increment MINOR version)

Is your feature request related to a problem?

Current testing has a number of problems:

• So-called "unit" testing is really more integration testing because it involves I/O rather than pure function testing. This leads to two points:
  • It may perhaps be better to convert the "unit" tests to bash scripts that check output. This would allow more transparency in what workflows are being tested. For example, one implementation of this is tests/scripts/globus_auth.bash.
  • Actual unit tests should test functions used by zstash rather than the actual zstash commands called from the command line.
• The current test framework uses unittest rather than pytest. I recall trying to modernize that in the past, but running into problems.
• The current test framework always uses the GitHubActions or conda/dev.yml Python version. We should simulate running zstash create with an older version of Python and/or zstash and then running zstash extract on a later version. For example, see the thread at Update Python support #384 (comment). It's a common use case that someone would be extracting data that was archived a while ago.

Describe the solution you'd like

Described above.

Describe alternatives you've considered

No response

Additional context

No response

[forsyth2]

Standard unit tests

Expand

# Perlmutter
cd ~/ez/zstash
git status
# No uncommited changes
git fetch upstream main
git checkout -b test-main-20251003 upstream/main
git log
# Last commit: Update docutils constraint (#386)
# Good, matches https://github.com/E3SM-Project/zstash/commits/main
nersc_conda
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zstash-main-20251003
conda activate zstash-main-20251003
pre-commit run --all-files
python -m pip install .
python -m unittest tests/test_*.py
# Ran 70 tests in 330.113s
# OK

# Chrysalis
cd ~/ez/zstash
git status
# No uncommited changes
git fetch upstream main
git checkout -b test-main-20251003 upstream/main
git log
# Last commit: Update docutils constraint (#386)
# Good, matches https://github.com/E3SM-Project/zstash/commits/main
lcrc_conda
rm -rf build
conda clean --all --y
conda env create -f conda/dev.yml -n zstash-main-20251003
conda activate zstash-main-20251003
pre-commit run --all-files
python -m pip install .
python -m unittest tests/test_*.py
# Ran 70 tests in 52.831s
# OK (skipped=32)

Extra tests

Expand

# Chrysalis
cd ~/ez/zstash/tests/scripts
# To start fresh with Globus:
# 1. Log into endpoints (LCRC Improv DTN, NERSC Perlmutter) at globus.org: File Manager > Add the endpoints in the "Collection" fields
# 2. To start fresh, with no consents: https://auth.globus.org/v2/web/consents > Manage Your Consents > Globus Endpoint Performance Monitoring > rescind all"
# Edit ~/ez/zstash/tests/scripts/globus_auth.bash
TRY_NUM=9
git diff # Make sure we edited on Chrysalis, not Perlmutter
./globus_auth.bash
# Success

# Chrysalis
cd ~/ez/zstash/tests/scripts
./symlinks.sh
# Success; note -- test is really just a check that it runs

# Perlmutter
cd ~/ez/zstash/tests/scripts
# Edit ~/ez/zstash/tests/scripts/follow_symlinks.sh
# conda_env=zstash-main-20251003
# source /global/homes/f/forsyth/miniforge3/etc/profile.d/conda.sh
# cd /global/homes/f/forsyth/ez/zstash/
# python -m pip install ../..
git diff # Make sure we edited on Perlmutter, not Chrysalis
./follow_symlinks.sh
# Success; note -- test is really just a check that it runs

# Perlmutter
cd /global/homes/f/forsyth/zstash/tests/
# Empty, no need for:
# rm -rf test_database_corruption
# Chrysalis
cd ~/ez/zstash/tests/scripts
./database_corruption.bash
# Success count: 25
# Fail count: 0

# Perlmutter
# Log into endpoints (Globus Tutorial Collection 1) at globus.org: File Manager > Add the endpoints in the "Collection" fields
cd ~/ez/zstash/tests/scripts_unit_tests
./test_ls_globus.bash
# Note -- test is really just a check that it runs
# Fails on final line because of a missing path
# Presumably, the hpss_path needs to be replaced with a different string
rm -rf zstash_test/

# Perlmutter
cd ~/ez/zstash/tests/scripts_unit_tests
./test_update_non_empty_hpss.bash
# Success; note -- test is really just a check that it runs

# Perlmutter
cd ~/ez/zstash
python -m unittest tests2/test_*.py
# Ran 4 tests in 3.514s
# OK

# Chrysalis
cd ~/ez/zstash
python -m unittest tests2/test_*.py
# Ran 4 tests in 5.943s
# OK

# Perlmutter
ls ~/ez/zstash/tests3
# This was Tony's testing for a PR

Test summary

The current state of zstash testing, with tests ordered by decreasing usability. Currently, python -m unittest tests/test_*.py is the primary test suite, with the other tests being extra and/or used in specific PRs.

	Has automatic assertions/checks	Runs on Perlmutter	Runs on Chrysalis	Works without test file edits	Specific PR?
cd ~/ez/zstash; python -m unittest tests/test_*.py HPSS tests	✅	✅		✅
cd ~/ez/zstash; python -m unittest tests/test_*.py non-HPSS tests	✅	✅	✅	✅
cd ~/ez/zstash; python -m unittest tests2/test_*.py	✅	✅	✅	✅
cd tests/scripts; ./globus_auth.bash (Replaced Globus unit test)	✅		✅	❌	#380
cd tests/scripts; ./database_corruption.bash	✅		✅	✅	#378
cd tests/scripts; ./symlinks.sh	❌		✅	✅	#343
cd tests/scripts_unit_tests; ./test_update_non_empty_hpss.bash	❌	✅		✅	#363
cd tests/scripts; ./follow_symlinks.sh	❌	✅		❌	#261
cd tests/scripts_unit_tests; ./test_ls_globus.bash	❌	✅		❌ (still needs editing)	#363
~/ez/zstash/tests3 (need to review testing instructions)	❓	❓	❓	❓	#355

[forsyth2]

Based on the primary rows, we can proceed with making rc1 but we should at some point resolve this issue of improving the tests... and also making them more uniform in terms of how to run, where to run from, etc.

[forsyth2]

#393 improved test structure, but it doesn't implement all the ideas listed above, so I'm re-opening this.

[forsyth2]

Giving Claude this subset of tests

tests/unit/test_functions.py
tests/integration/python_tests/group_by_workflow/base.py
tests/intergration/python_tests/group_by_workflow/test_cache_fs.py
tests/integration/python_tests/group_by_command/base.py
tests/integration/python_tests/group_by_command/test_check.py 
tests/integration/bash_tests/run_from_any/globus_auth.bash

resulted in:

---

Claude's full list of suggestions to improving tests

Looking at your test suite, I can see a well-intentioned organizational structure, but there are some improvements that could make the categorization clearer and reduce complexity. Let me analyze what you have:

Current Organization Assessment

✅ What's Working Well:

1. tests/unit/test_functions.py - This is a true unit test. It tests parse_tars_option() in isolation with no I/O, no subprocess calls, no file system operations. Perfect.

2. Separation of concerns - You've already separated integration tests by workflow vs command, which shows good thinking.

❌ What Needs Reorganization:

The "integration" tests are actually a mix:

1. test_cache_fs.py and test_check.py - These are actually end-to-end (E2E) tests, not integration tests. They:

   • Create real directories and files
   • Run the full zstash command via subprocess
   • Test the entire application behavior from CLI to file system
   • Use external dependencies (HPSS, potentially)

2. globus_auth.bash - This is a manual system test requiring human interaction. It's more of a test script/procedure than an automated test.

Recommended Reorganization

tests/
├── unit/                          # Pure unit tests (no I/O, no external deps)
│   ├── test_functions.py          # ✅ Already correct
│   ├── test_tar_parsing.py        # Parse logic without I/O
│   └── test_validation.py         # Input validation logic
│
├── integration/                   # Tests that integrate 2-3 components
│   ├── test_database_operations.py    # Test DB interactions
│   ├── test_archive_creation.py       # Test tar creation logic (mock file system)
│   └── test_transfer_logic.py         # Test transfer without actual HPSS/Globus
│
├── e2e/                           # Full system tests
│   ├── local/                     # Tests using --hpss=none
│   │   ├── test_create_extract.py
│   │   ├── test_symlinks.py      # Your current test_cache_fs.py content
│   │   └── test_check_verify.py  # Your current test_check.py content
│   │
│   ├── hpss/                      # Tests requiring HPSS
│   │   ├── test_hpss_transfer.py
│   │   └── test_hpss_retry.py
│   │
│   └── globus/                    # Tests requiring Globus
│       └── test_globus_transfer.py
│
└── manual/                        # Manual test procedures
    ├── globus_auth_procedure.bash  # Your globus_auth.bash
    └── README.md                   # Instructions for manual tests

Specific Issues with Current Tests

1. Overly Complicated Base Classes

Both base.py files have grown too large and do too much:

# ❌ Current: TestZstash does EVERYTHING
class TestZstash(unittest.TestCase):
    def setUp(self): ...
    def tearDown(self): ...
    def setupDirs(self): ...
    def create(self): ...
    def add_files(self): ...
    def extract(self): ...
    def assert_source_unchanged(self): ...
    # ... many more methods

Better approach:

# ✅ Separate concerns
class FileSystemFixture:
    """Helper for creating test file structures"""
    @staticmethod
    def create_test_structure(base_path):
        # Just create files, no assertions
        ...

class ZstashCommandRunner:
    """Helper for running zstash commands"""
    @staticmethod
    def run_create(path, options):
        # Just run command, return output
        ...

class E2ETestBase(unittest.TestCase):
    """Minimal base with setup/teardown only"""
    def setUp(self):
        self.temp_dir = tempfile.mkdtemp()
    
    def tearDown(self):
        shutil.rmtree(self.temp_dir)

2. globus_auth.bash Complexity

This 400+ line bash script with manual confirmation prompts is problematic:

# ❌ Current approach
if ! confirm "Did you only have to paste an auth code once?"; then
    echo "Single-authentication test failed"
    exit 1
fi

This should either be:

1. Automated using mock Globus responses
2. Documented as a manual procedure in a runbook (not executable test)
3. Moved to a separate CI workflow that can handle interactive testing

3. Mixed Test Levels in Same File

test_check.py mixes what should be different test levels:

class TestCheck(TestZstash):
    def testCheck(self):  # E2E test - runs full command
        ...
    
    def testCheckVerboseMismatch(self):  # E2E test
        ...
    
    def testCheckTars(self):  # E2E test with specific option
        ...

These are all E2E tests but could benefit from pyramid testing:

# Unit test
def test_tar_range_parsing():
    assert parse_tar_range("000001-000003") == ["000001", "000002", "000003"]

# Integration test (mocked file system)
def test_check_reads_correct_tars(mock_fs):
    checker = TarChecker(mock_fs)
    result = checker.check_range("000001-000003")
    assert result.files_checked == 3

# E2E test (real files)
def test_check_command_end_to_end(temp_dir):
    # Full command with real I/O
    output = run_zstash_check(...)

Recommendations

1. Extract More Unit Tests

Look for logic that can be tested without I/O:

# From your base.py - these could be unit tested:
- check_strings() logic
- compare() function
- Tar name parsing
- Path manipulation
- Option validation

2. Create True Integration Tests

Test component interactions with controlled dependencies:

# Integration test example
def test_database_and_archive_integration(tmp_path):
    """Test that database correctly tracks archived files"""
    db = Database(tmp_path / "index.db")
    archiver = TarArchiver(tmp_path / "cache")
    
    # Add file to archive
    archiver.add_file("test.txt", "content")
    db.record_file("test.txt", archiver.last_tar_md5)
    
    # Verify integration
    assert db.get_file_tar("test.txt") == archiver.last_tar_name

3. Simplify E2E Tests

Use pytest fixtures and helpers:

@pytest.fixture
def zstash_archive(tmp_path):
    """Fixture that creates a basic zstash archive"""
    archive_path = tmp_path / "archive"
    # Create test structure
    # Run zstash create
    return archive_path

def test_extract_preserves_symlinks(zstash_archive):
    """E2E test with fixture"""
    extract_path = zstash_archive.parent / "extracted"
    run_zstash_extract(zstash_archive, extract_path)
    
    assert (extract_path / "symlink").is_symlink()

4. Mark Tests by Type and Requirements

@pytest.mark.unit
def test_parse_tars_option():
    ...

@pytest.mark.integration
def test_database_operations():
    ...

@pytest.mark.e2e
@pytest.mark.slow
def test_full_create_extract():
    ...

@pytest.mark.e2e
@pytest.mark.requires_hpss
@pytest.mark.skip("Requires HPSS access")
def test_hpss_transfer():
    ...

Summary

Your tests are functionally working but the organization could be clearer:

• ✅ You have one good unit test
• ❌ Your "integration" tests are actually E2E tests
• ❌ Base classes do too much
• ❌ Manual bash test should be documentation/procedure
• ✅ The separation by workflow vs command shows good thinking

Key improvements:

1. Extract more unit tests from parsing/validation logic
2. Create true integration tests with mocked I/O
3. Move current "integration" tests to e2e/ folder
4. Simplify base classes using composition over inheritance
5. Convert bash script to either automated mock tests or manual documentation