Enhanced some poor tests in `test_all_plugins.py`

[Muhammad-Rebaal]

Hi @khsrali !

I've enhance the tests in the Todo's and solved some of them except those that you mentioned get, put, and copy methods which would be later implemented by you in the issue #6727 . If there is anything else that should be changed,let me know .

Thank You!

[khsrali]

Thanks for your contribution @Muhammad-Rebaal

Some of tests are failing, see here: https://github.com/aiidateam/aiida-core/actions/runs/13718527513/job/38368567821?pr=6787

You may also try to run them locally, to make sure everything passes, via: pytest tests/transports/test_all_plugins.py -s

keep in mind, you may need to have your ssh localhost configured, passwordless.

[Muhammad-Rebaal]

Thanks for your contribution @Muhammad-Rebaal

Some of tests are failing, see here: https://github.com/aiidateam/aiida-core/actions/runs/13718527513/job/38368567821?pr=6787

You may also try to run them locally, to make sure everything passes, via: pytest tests/transports/test_all_plugins.py -s

keep in mind, you may need to have your ssh localhost configured, passwordless.

Your Welcome @khsrali ! Yeah sure ! I'll keep in my mind and will run the tests locally so that everything is working perfectly fine. Thank You !

[khsrali]

Feel free to ping me again, once you managed to have all tests pass. Thanks!

[Muhammad-Rebaal]

Feel free to ping me again, once you managed to have all tests pass. Thanks!

Hey @khsrali ! Just struggling with the ssh part as it was asking password when connecting that on SSH Server on window but I've already did the part like creating key and copying that into the authorized key if you know something about it let me know and the test is working if I skip the ssh part if that's not the problem or it is essential then I'll check the test the script by skipping the SSH part and push the code without this code as it is only for my pc to skip ssh , so please let me know .

@pytest.fixture( scope='function', params=[ name for name in entry_point.get_entry_point_names('aiida.transports') if (sys.platform == 'win32' and name == 'core.local') or # Windows: only local (sys.platform != 'win32' and name.startswith('core.')) # Other platforms: all core ], )

Thank You !

[khsrali]

Hi @Muhammad-Rebaal We have several transport plugin: core.ssh core.local core.ssh_auto core.ssh_async

The test suit has to run and pass for all of them. I don't know how to configure password-less ssh localhost on windows. And even if you do, I'm not sure if tests should all pass on windows as we don't officially support it. So here is the steps to be taken, if you want to use windows:

1. configure password-less ssh localhost -- ask the how to from chatgpt or similar
2. run pytest test_all_plugins.py -s for aiida-core main branch EXCLUDING YOUR CHANGES!
3. does everything pass? -> yes: good! now run the test again with changes and start debugging -> no: The best solution: maybe consider installing a linux distro :upside_down_face: -> no: Not the best solution: read what failed here, fix them and push again to this PR. Repeat this painful procedure until all tests pass.

[Muhammad-Rebaal]

Hey @khsrali ! Hope you are fine ! The linux really worked and to be honest its a hard one😅, but somehow I've passed the tests and get things done , so could you take a look at it here's a reference picture and if there is anything else let me know , I'm eagerly waiting for your response ?

Thank You !

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 78.21%. Comparing base (83454c7) to head (15eeb00). :warning: Report is 132 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6787      +/-   ##
==========================================
- Coverage   78.31%   78.21%   -0.09%     
==========================================
  Files         566      566              
  Lines       42762    42762              
==========================================
- Hits        33484    33444      -40     
- Misses       9278     9318      +40     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Muhammad-Rebaal]

hi @Muhammad-Rebaal, the PR looks quite confusing, why there are so many changes that are in related? Are you working from the main branches?

To start, I think it is better to provide changes by focusing on one test that you think you can improve.

Hi @unkcpz !

First of all Thank you for the response ! Yes I started with the main branch , a little early I think before @khsrali contribution , that's why I face the issue of path and then convert it into str. Do I've to start from the beginning ? or after some tuning things can be fixed ?

[unkcpz]

that's why I face the issue of path and then convert it into str. Do I've to start from the beginning ? or after some tuning things can be fixed ?

No, you don't need to start from beginning, please just rebase.

[Muhammad-Rebaal]

Hi @unkcpz ,

After rebasing git rebase main as suggested, I've noticed that the issue with Path objects in the SSH transport persists. When I try to test this function test_makedirs() with the command pytest tests/transports/test_all_plugins.py::test_makedirs -vpytest tests/transports/test_all_plugins.py::test_makedirs -v

   def test_makedirs(custom_transport, tmpdir):
      """Verify the functioning of makedirs command and its handling of various edge cases"""
      with custom_transport as transport:
     # Test 1: Basic mkdir creates a single directory
     _scratch = Path(tmpdir / 'sampledir')
      transport.mkdir(_scratch)
      assert _scratch.exists()
      assert _scratch.is_dir()  # Using is_dir() for Path objects

    # Test 2: makedirs creates nested directories
      _scratch = tmpdir / 'sampledir2' / 'subdir'
       transport.makedirs(_scratch)
       assert _scratch.exists()
       assert _scratch.isdir()  # Using isdir() for LocalPath objects

    # Test 3: makedirs with existing parent directory still works
       _scratch = tmpdir / 'sampledir2' / 'another_subdir'
       transport.makedirs(_scratch)
       assert _scratch.exists()
      assert _scratch.isdir()  # Using isdir() for LocalPath objects

    # Test 4: Raises OSError when directory already exists
       with pytest.raises(OSError):
          transport.makedirs(tmpdir / 'sampledir2')
       with pytest.raises(OSError):
          transport.mkdir(tmpdir / 'sampledir')

    # Test 5: Test with deeply nested directories (3+ levels)
       _scratch = tmpdir / 'deep' / 'nested' / 'directory' / 'structure'
       transport.makedirs(_scratch)
       assert _scratch.exists()
       assert _scratch.isdir()  # Using isdir() for LocalPath objects

    # Test 6: Test that parent directories were created properly
       assert (tmpdir / 'deep').exists()
       assert (tmpdir / 'deep').isdir()  # Using isdir() for LocalPath objects
       assert (tmpdir / 'deep' / 'nested').exists()
      assert (tmpdir / 'deep' / 'nested').isdir()  # Using isdir() for LocalPath objects
      assert (tmpdir / 'deep' / 'nested' / 'directory').exists()
      assert (tmpdir / 'deep' / 'nested' / 'directory').isdir()  # Using isdir() for LocalPath objects

    # Test 7: Test that makedirs preserves existing directory permissions
    # Create directory with specific permissions
      _permission_dir = tmpdir / 'permission_test'
      transport.mkdir(_permission_dir)
      transport.chmod(_permission_dir, 0o755)

    # Create subdirectory with makedirs
      _subdir = _permission_dir / 'subdir'
      transport.makedirs(_subdir)
      assert _subdir.exists()

    # Parent directory permissions shouldn't be changed
      assert transport.get_mode(_permission_dir) == 0o755

I've also try to implement the explicit string conversion in the mkdir method in the ssh.py file :

 def mkdir(self, path: TransportPath, ignore_existing: bool = False):
     path = str(path)

    if ignore_existing and self.isdir(path):
    return

    try:
      self.sftp.mkdir(str(path))  # Added explicit conversion here
   except OSError as exc:
     # Error handling...

However, the test still fails with the same error:

When you have a moment, I’d appreciate your insights on this—perhaps there’s something I might have missed.

Thank You !

[GeigerJ2]

Hi all, what's the current state here? We're currently in the process of cleaning up the open PRs. Can it be finalized and merged soonish or should we close it for now?

[khsrali]

Effort to review does not pay of the changes