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.

Sign up for GitHub

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

Jump to bottom

Mock pubmed 557 #567

Merged
merged 5 commits into from
Aug 30, 2024
Merged

Mock pubmed 557 #567

merged 5 commits into from
Aug 30, 2024

Conversation

knirirr
Copy link

@knirirr knirirr commented Aug 27, 2024

This mocks two tests as described in #557

@knirirr knirirr requested a review from proccaserra August 27, 2024 10:31
@knirirr
Copy link
Author

knirirr commented Aug 27, 2024

@proccaserra there are some test failures here and locally in other parts of the code; IIRC these are ones we have discussed before where counts have changed, e.g.

======================================================================
FAIL: test_b_ii_s_3 (isatab.validate.test_core.TestValidators.test_b_ii_s_3)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/isa-api/tests/isatab/validate/test_core.py", line 20, in test_b_ii_s_3
    self.assertEqual(len(r['warnings']), 12)
AssertionError: 5 != 12

@knirirr knirirr linked an issue Aug 27, 2024 that may be closed by this pull request
mock pubmed API call in test #557
Closed
@knirirr knirirr changed the base branch from master to develop August 27, 2024 10:43
proccaserra
proccaserra reviewed Aug 27, 2024
View reviewed changes
tests/utils/test_isatools_utils.py Outdated
@@ -82,11 +83,12 @@ def test_get_ontology(self):
ontology_source = ols.get_ols_ontology('efo')
self.assertIsInstance(ontology_source, OntologySource)
self.assertEqual(ontology_source.name, 'efo')
self.assertIn("https://www.ebi.ac.uk/ols", ontology_source.file)
self.assertIn("/api/ontologies/efo?lang=en", ontology_source.file)
self.assertIsInstance(ontology_source.version, str)
self.assertEqual(
Copy link
Member

Choose a reason for hiding this comment

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

@knirirr I recall that we coded it this way with @terazus to guard against version change, ( which makes the assumption that the service would keep the pattern and can not be guaranteed)

proccaserra
proccaserra reviewed Aug 27, 2024
View reviewed changes
Copy link
Member

@proccaserra proccaserra left a comment

Choose a reason for hiding this comment

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

apart from the comment on ols , the rest looks fine to me. thx @knirirr

@knirirr
Copy link
Author

knirirr commented Aug 27, 2024

Thanks. I'm not clear on how the OLS part should look, I'm afraid.
I'm also assuming that the 'develop' branch is being used as this appears to be ahead of master. I've merged that into my branch and changed this PR to develop instead of master.

@coveralls
Copy link

Coverage Status

coverage: 81.251% (-0.2%) from 81.404%
when pulling 0b4afe6 on mock_pubmed_557
into 00c2f63 on develop.

@knirirr knirirr merged commit 11913a7 into develop Aug 30, 2024
10 checks passed
@knirirr knirirr deleted the mock_pubmed_557 branch August 30, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@proccaserra proccaserra proccaserra left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mock pubmed API call in test
3 participants
@knirirr @coveralls @proccaserra