search

faucetsdn / faucetagent

gNMI agent for faucet configuration
4 stars 2 forks source link

add mininet dependency for tests #2

Closed cglewis closed 5 years ago

gizmoguy commented 5 years ago

Thanks @cglewis I guess you found this before we announced it :)

lantz commented 5 years ago

I don't understand this change.

The dependencies are installed by dependencies.sh, so why would we want to add an additional installation step in the makefile?

cglewis commented 5 years ago

@lantz if the system you're running the test on doesn't already have mininet installed it fails. this change ensures that mininet is installed before running the tests.

I didn't put it in the dependencies, since it appears to only be needed for testing, not for the actual agent.

lantz commented 5 years ago

The test dependencies are installed in .travis.yml but I think this is an argument for creating a test dependency script rather than putting it in the makefile and reinstalling on every make test.

cglewis commented 5 years ago

I don't have a strong preference either way, was just trying to make it a repeatable process for testing without travis.

lantz commented 5 years ago

Unfortunately, installing mininet with pip3 isn't sufficient, because it doesn't install the mnexec helper tool which mininet needs in order to run.

lantz commented 5 years ago

@cglewis I created #3 to address this - if you have no objections, then we can merge it in.

cglewis commented 5 years ago

looks good to me, thanks for clarifying that and fixing :)