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

nixosTests.networkingProxy: port to Python #78239

Merged

Conversation

andrew-d
Copy link
Contributor

Motivation for this change

#72828

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @tfc @ardumont

@andrew-d andrew-d changed the title nixosTest.networking-proxy: port to Python nixosTests.networking-proxy: port to Python Jan 22, 2020
@andrew-d andrew-d force-pushed the andrew/networking-proxy-tests-python branch from ab0a080 to e818a1a Compare January 22, 2020 05:31
@andrew-d
Copy link
Contributor Author

@tfc - Yep, can do for both. As a general question: for these sorts of "porting tests" PRs, would it be better to do a mechanical line-by-line translation (like I did here), or a more general rewrite?

@tfc
Copy link
Contributor

tfc commented Jan 22, 2020

@tfc - Yep, can do for both. As a general question: for these sorts of "porting tests" PRs, would it be better to do a mechanical line-by-line translation (like I did here),

this is what i did when the whole project began, as i don't understand how all of the tested services really work. Then a 1:1 port was the minimal feasible thing to do.

or a more general rewrite?

If you are the maintainer of the service, the test, or simply happen to know how the tested service really works and ought to be tested, a rewrite that makes it better/more readable/more structured is always welcome!

nixos/tests/networking-proxy.nix Outdated Show resolved Hide resolved
nixos/tests/networking-proxy.nix Outdated Show resolved Hide resolved
nixos/tests/networking-proxy.nix Outdated Show resolved Hide resolved
@andrew-d andrew-d force-pushed the andrew/networking-proxy-tests-python branch from e818a1a to 8507f84 Compare January 23, 2020 00:03
@andrew-d
Copy link
Contributor Author

@flokli @tfc - Okay, since improvements are okay, I rewrote the tests to use more modern Python, remove duplication, and use proper assertions.

@andrew-d
Copy link
Contributor Author

@flokli @tfc - ping! Anything I can do to keep this moving forward?

@tfc
Copy link
Contributor

tfc commented Jan 30, 2020

@andrew-d sorry didn't see the latest changes. I think @flokli is pretty busy at the time.

cc @worldofpeace

@tfc
Copy link
Contributor

tfc commented Jan 30, 2020

@GrahamcOfBorg test networkingProxy

@worldofpeace worldofpeace added this to the 20.03 milestone Jan 31, 2020
@worldofpeace worldofpeace changed the title nixosTests.networking-proxy: port to Python nixosTests.networkingProxy: port to Python Jan 31, 2020
@worldofpeace worldofpeace force-pushed the andrew/networking-proxy-tests-python branch from ec15ea0 to 36d1141 Compare January 31, 2020 00:13
@worldofpeace
Copy link
Contributor

Fixed commit message.

@worldofpeace worldofpeace merged commit b36f4c8 into NixOS:master Jan 31, 2020
anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
…tests-python

nixosTests.networkingProxy: port to Python
@andrew-d andrew-d deleted the andrew/networking-proxy-tests-python branch March 29, 2020 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants