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

Add proto='tcp' kwarg to port_for #32

Closed
wants to merge 1 commit into from
Closed

Add proto='tcp' kwarg to port_for #32

wants to merge 1 commit into from

Conversation

vicyap
Copy link

@vicyap vicyap commented Nov 19, 2018

I have a use case where I need to get the udp port for a service.

@@ -4,5 +4,5 @@ coverage
docutils
flake8
mock
pytest
pytest<4.0.0
Copy link
Author

Choose a reason for hiding this comment

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

I needed to version lock pytest because I was getting the following error.

RemovedInPytest4Warning: Fixture "docker_services" called directly. Fixtures are not meant to be called directly, are created automatically when test functions request them as parameters. See https://docs.pytest.org/en/latest/fixture.html for more information.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I unpinned this requirement.

@butla
Copy link

butla commented Nov 19, 2018

That's a warning, not an error. I think we can live with it for now. We'd need to call the fixtures with one of pytest functions to not invoke that warning. I can't recall that for now, but it's definitely somewhere in the docs related to testing plugins.

Also, putting that range in requirements.in might give the impression that this plugin somehow doesn't support the new pytest.

@butla
Copy link

butla commented Nov 22, 2018

My sample tests (https://github.com/butla/experiments/tree/master/functional_tests) on your branch and the code seems OK.

@vicyap Could you just squash (git rebase -i HEAD~4 while being on the HEAD of your branch) the branch into a single commit?

@AndreLouisCaron Looks good to me, if that matters :)

@vicyap
Copy link
Author

vicyap commented Nov 27, 2018

Alright, @butla , I squashed my commits

@AndreLouisCaron pending your approval

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.

None yet

2 participants