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

Upgrade cgap-portal to pytest 7.2 (C4-183) #687

Draft
wants to merge 39 commits into
base: master
Choose a base branch
from
Draft

Conversation

netsettler
Copy link
Contributor

@netsettler netsettler commented Jan 31, 2023

  • Backport pytest 7.2 support from Fourfront
  • Move to poetry 1.3.2
  • Remove dependence in a pinned version of setuptools

…otes in comments. No real functional change.
…ng in src/encoded/tests/test_create_mapping.py.
…of tests into variables so that they are shared in ways that are easier to understand.
@netsettler netsettler changed the title Upgrade to pytest 7.2 Upgrade to pytest 7.2 (C4-1003) Mar 14, 2023
@netsettler netsettler changed the title Upgrade to pytest 7.2 (C4-1003) Upgrade to pytest 7.2 (C4-183) Mar 14, 2023
pyproject.toml Outdated
jmespath = "0.9.0"
jsonschema_serialize_fork = "^2.1.1"
keepalive = "0.5"
# keepalive = "0.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we like to delete these entirely rather than keeping commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, when I'm sometimes turning things on and off, I leave them for a while and delete them later, but I think this can reasonably go away before I merge.

Copy link
Contributor

@willronchetti willronchetti left a comment

Choose a reason for hiding this comment

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

These changes generally are fine but I think we should look further at the file processed test as to why it is failing in GA

@@ -180,8 +162,11 @@ def test_create_mapping_on_indexing(app, setup_and_teardown, testapp, registry,
assert compare_against_existing_mapping(es, namespaced_index, item_type, item_record, True)


@pytest.mark.flaky
def test_file_processed_detailed(app, setup_and_teardown, testapp, indexer_testapp, project, institution, file_formats):
@pytest.mark.broken # Doesn't work on GitHub Actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we look into this further? What error is occurring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look at that, yes.

@netsettler netsettler changed the title Upgrade to pytest 7.2 (C4-183) Upgrade cgap-portal to pytest 7.2 (C4-183) Mar 14, 2023
@netsettler netsettler marked this pull request as draft March 30, 2023 00:41
@netsettler netsettler added the salvage Not planned for merge but may contain scraps that need rescue label Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
salvage Not planned for merge but may contain scraps that need rescue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants