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 more network tests #2980

Merged
merged 6 commits into from Feb 25, 2022
Merged

fix more network tests #2980

merged 6 commits into from Feb 25, 2022

Conversation

trichter
Copy link
Member

@trichter trichter commented Feb 8, 2022

I fixed some more of the network tests. I skipped this one clients/iris doctest which I did not get to pass. The same thing is tested in the regular test suite.

This can be merged as is. Someone else needs to figure out those earthworm stuff. I have no clue about it.

@trichter trichter added testing issues generally related to our testing setup / infrastructure test_network tell github actions to also run network tests for this PR ready for review PRs that are ready to be reviewed to get marked ready to merge labels Feb 8, 2022
@d-chambers
Copy link
Member

Looks like the non-network tests are failing due to new warnings cropping up. Perhaps not a big deal, should be easy to fix.

@d-chambers
Copy link
Member

Also, certainly not required but if you are feeling ambitious converting these test cases to pytest style is something we should do eventually.

@d-chambers
Copy link
Member

Looks like you are addressing the failures in #2978

@trichter
Copy link
Member Author

trichter commented Feb 9, 2022

Also, certainly not required but if you are feeling ambitious converting these test cases to pytest style is something we should do eventually.

I dont feel ambitious enough at the moment :)

@trichter trichter added this to the 1.3.0 milestone Feb 11, 2022
@@ -234,14 +234,15 @@ def test_resp(self):
t2 = UTCDateTime("2008-001T00:00:00")
result = client.resp("IU", "ANMO", "00", "BHZ", t1, t2)
self.assertIn(b'B050F03 Station: ANMO', result)
# Exception: No response data available
Copy link
Member

Choose a reason for hiding this comment

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

Maybe choose a different response so that we can still test this part of the code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Still two requests left...

The site from which examples were taken still mentions this query. Maybe we can use this one.

@trichter
Copy link
Member Author

@d-chambers Feel free to finish this one :)

@trichter
Copy link
Member Author

I implemented one more test in clients.iris module as suggested. I think this can be merged.

@trichter trichter merged commit ffede66 into master Feb 25, 2022
@trichter trichter deleted the network_tests branch February 25, 2022 12:56
@trichter trichter removed test_network tell github actions to also run network tests for this PR ready for review PRs that are ready to be reviewed to get marked ready to merge labels Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing issues generally related to our testing setup / infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants