Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: backport typing #69

Closed
wants to merge 2 commits into from
Closed

fix: backport typing #69

wants to merge 2 commits into from

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Apr 24, 2023

The Parser class was made available at pytest.Parser only for pytest 7.

See pytest-dev/pytest@538b5c2

The `Parser` class was made available at `pytest.Parser` only for pytest 7.

See pytest-dev/pytest@538b5c2
@ltalirz
Copy link
Member Author

ltalirz commented Apr 24, 2023

It appears the a number of tests are broken (I suspect due to changes on the aiida-core side)

============================= test session starts ==============================
platform linux -- Python 3.9.16, pytest-7.3.1, pluggy-1.0.0
rootdir: /home/runner/work/aiida-testing/aiida-testing
configfile: pyproject.toml
plugins: datadir-1.4.1, aiida-testing-0.1.0.dev1, mock-3.10.0
collected 20 items

archive_cache/test_archive_cache.py ...FFF                               [ [30](https://github.com/aiidateam/aiida-testing/actions/runs/4786993838/jobs/8511632543?pr=69#step:6:31)%]
mock_code/test_diff.py ..F......                                         [ 75%]
mock_code/test_ignore_paths.py ..                                        [ 85%]
mock_code/test_isolated.py ...                                           [100%]

Most of them are in archive cache; one is also in test_diff
@janssenhenning for info

2023-04-24T13:38:42.7155597Z     def test_broken_code(mock_code_factory, generate_diff_inputs):
2023-04-24T13:38:42.7155852Z         """
2023-04-24T13:38:42.7156142Z         Check that the mock code works also when no executable is given, if the result exists already.
2023-04-24T13:38:42.7156424Z         """
2023-04-24T13:38:42.7156637Z         mock_code = mock_code_factory(
2023-04-24T13:38:42.7156912Z             label='diff-broken',
2023-04-24T13:38:42.7157162Z             data_dir_abspath=TEST_DATA_DIR,
2023-04-24T13:38:42.7157417Z             entry_point=CALC_ENTRY_POINT,
2023-04-24T13:38:42.7157748Z             ignore_paths=('_aiidasubmit.sh', 'file?.txt')
2023-04-24T13:38:42.7157984Z         )
2023-04-24T13:38:42.7158146Z     
2023-04-24T13:38:42.7158333Z         res, node = run_get_node(
2023-04-24T13:38:42.7158647Z             CalculationFactory(CALC_ENTRY_POINT), code=mock_code, **generate_diff_inputs()
2023-04-24T13:38:42.7158925Z         )
2023-04-24T13:38:42.7159116Z         assert node.is_finished_ok
2023-04-24T13:38:42.7159346Z >       check_diff_output(res)
2023-04-24T13:38:42.7159487Z 
2023-04-24T13:38:42.7159587Z mock_code/test_diff.py:102: 
2023-04-24T13:38:42.7159841Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2023-04-24T13:38:42.7159980Z 
2023-04-24T13:38:42.7160715Z result = {'diff': <SinglefileData: uuid: 0b72fbeb-3d8a-4fc6-946a-8a853e882d6b (pk: 88)>, 'remote_folder': <RemoteData: uuid: b7...-2071-41c0-8959-0431aedce2fb (pk: 86)>, 'retrieved': <FolderData: uuid: cadbb10b-3dbd-4207-b3cb-178b41c63f91 (pk: 87)>}
2023-04-24T13:38:42.7161100Z 
2023-04-24T13:38:42.7161207Z     def check_diff_output(result):
2023-04-24T13:38:42.7161516Z         """
2023-04-24T13:38:42.7161785Z         Checks the result from a diff calculation against a reference.
2023-04-24T13:38:42.7162024Z         """
2023-04-24T13:38:42.7162225Z         diff_res_lines = tuple(
2023-04-24T13:38:42.7162625Z             line.strip() for line in result['diff'].get_content().splitlines() if line.strip()
2023-04-24T13:38:42.7162886Z         )
2023-04-24T13:38:42.7163082Z >       assert diff_res_lines == (
2023-04-24T13:38:42.7163400Z             "1,2c1", "< Lorem ipsum dolor..", "<", "---",
2023-04-24T13:38:42.7163691Z             "> Please report to the ministry of silly walks."
2023-04-24T13:38:42.7163916Z         )
2023-04-24T13:38:42.7164235Z E       AssertionError: assert () == ('1,2c1', '< ...silly walks.')
2023-04-24T13:38:42.7164617Z E         Right contains 5 more items, first extra item: '1,2c1'
2023-04-24T13:38:42.7164864Z E         Full diff:
2023-04-24T13:38:42.7165055Z E           (
2023-04-24T13:38:42.7165240Z E         +  ,
2023-04-24T13:38:42.7165454Z E         -  '1,2c1',
2023-04-24T13:38:42.7165720Z E         -  '< Lorem ipsum dolor..',
2023-04-24T13:38:42.7165959Z E         -  '<',
2023-04-24T13:38:42.7166164Z E         -  '---',
2023-04-24T13:38:42.7166475Z E         -  '> Please report to the ministry of silly walks.',
2023-04-24T13:38:42.7166715Z E           )

@janssenhenning
Copy link
Collaborator

janssenhenning commented Apr 24, 2023

@ltalirz The archive tests should be fixed by adding the metadata_inputs attribute to the ignored attributes, i.e. in .aiida-testing-config.yml

archive_cache:
  ignore:
    calcjob_attributes:
      #These attributes have to be ignored to be able to run the export cache tests
      #while reusing the same test archive, migrating it to the needed version,
      #since they are only present in newer versions
      #The test archives have version 0.8
      - environment_variables_double_quotes #This option was introduced in aiida-core 2.0
      - submit_script_filename #This option was introduced in aiida-core 1.2.1 (archive version 0.9)
      - metadata_inputs # Added in aiida-core 2.3.0

I can't commit to the branch directly if you want you can commit the change directly or I can open a separate PR for it.

@ltalirz
Copy link
Member Author

ltalirz commented Apr 24, 2023

Ah great, thanks for looking into this @janssenhenning !

Added to the ignore list

P.S. Also just invited you as a contributor to the repo

@ltalirz
Copy link
Member Author

ltalirz commented Apr 24, 2023

that did the trick - great

archive_cache/test_archive_cache.py ......                               [ 30%]
mock_code/test_diff.py ..F......                                         [ 75%]
mock_code/test_ignore_paths.py ..                                        [ 85%]
mock_code/test_isolated.py ...                                           [100%]

@danielhollas
Copy link
Collaborator

Instead of going with this PR we could also simply bump the required pytest version. I don't see much of a reason to support anything older? (But I still need to explore where this package is used in the wild). But the fact that nobody complained about this seems to indicate it would be fine to bump the version.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 17, 2024

sure, that's fine as well

@danielhollas
Copy link
Collaborator

Superseded by #73 where I bumped minimum pytest version to 7.0 (it still supports Python 3.7 so its fine).

@danielhollas danielhollas deleted the fix/backport-typing branch November 19, 2024 00:41
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.

3 participants