meenaso, i didn't realize we have no fallback when blkid doesn't work in ds-identify, so here's a bug: https://bugs.launchpad.net/cloud-init/+bug/190117410:15
ubot5Ubuntu bug 1901174 in cloud-init "ds-identify cannot identify disk-based resources on FreeBSD" [Undecided,New]10:15
=== cpaelzer__ is now known as cpaelzer
=== dionysus70 is now known as dionysus69
minimalCan anyone help with a issue related to testcases? I'm working on a PR and have written a testcase for it but getting the weird situation where the RHEL sysconfig output appears to be JSON rather than plain text and so doesn't match the expected output14:32
meenait's not json, it's the object14:49
meenaminimal, it means you're comparing a serialised version of thing with an unserialised one14:58
minimalmeena: I've just using the pre-existing functions in test_net.py that compare expected output with actual output15:13
minimalI think I'm just getting confused by the failure output and its actually worked as intended15:28
minimalI'm still a bit confused about the testing layout in test_net.py - I expected to be able to define an "input" network config v2 YAML and the expected ENI, Netplan, and sysconfig for it in one place and have all outputs tested in on go but now I realise there are separate classes for testing each of these output formats15:32
minimalso seems I need to write separate testcases for each of the output types15:32
rharperminimal: yes, for a new scenario in test_net.py,  you have an input, which is ingested into internal network_state; and then for each distro/renderer, there's an expected output;  so it sounds like you're on the right track;   alternatively you might chat with Odd_Bloke to see if there are suggestions on switching to a pytest parametrized style which could significantly improve how those tests get written;15:38
minimalrharper: what's confused me is that in test_net.py there's "dhcpv6_stateless" which contains expected_eni, expected_netplan, expected_sysconfig_opensuse, and expected_sysconfig_rhel which I copied/pasted (as well as the function test_dhcpv6_stateless_config that uses it) as a basic for my new testcase but when I ran tox only expected_sysconfig_rhel appears to be tested - none of the other outputs are checks so why are they defined/present.15:42
rharperminimal: a diff would help; but test inputs/outputs data are reused in differnt test Classes in that file15:43
rharpertest_net.py is a *beast* and it's easy to not see where all of the call sites are due to the repeated pattern of the classes15:43
minimalyeah, I'm finding it hard to figure out how it works when its 5000+ lines long15:44
rharperminimal: exactly =?15:47
minimalseems I need to put the function for ENI in class TestEniRoundTrip, not in class TestEniNetRendering as I was doing.....doh!15:48
rharpera small refactor could take the defined configs and put them in tests/data/test_net_configs.py;  and then each of the classes in test_net, could be test_net_classname.py and import what they needed from data/test_net_configs.py15:48
minimalcertainly seems like refactoring is badly needed. I'm trying to do the right thing and create a testcase for functionality that's been broken for how knows how long as its not currently tested.15:50
minimaltime to reach for some headache tablets........... ;-)15:50
Odd_BlokeYeah, I think those tests have been structured well in that the test data and testing code _are_ separate, but that approach still leads to a lot of boilerplate in "vanilla" Python unit testing (which I agree parameterisation could address).  Even "just" a refactor to split them into separate files as suggested would help a great deal, I think.16:16
minimalhave figured it out now for eni and netplan so now need to make changes to sysconfig.py before I can test that16:24
minimalOdd_Bloke: ideally it would be nice to have a generic test function that then behind the scenes calls one-or-more or all of the specific output functions16:26
minimalso for "dhcp" for example the test_dhcp function would call test_netplan_dhcp, test_eni_dhcp etc but the test_dhcp would have an arg to let you select some of the outputs if you did'nt want them all called16:27
Odd_Blokeminimal: Are you familiar with pytest parameterisation (https://docs.pytest.org/en/stable/parametrize.html)?  It allows you to programmatically generate multiple instances of the same test with different inputs: I think this would map onto the current dicts we already have pretty well.16:47
Odd_BlokeSo instead of (e.g.) TestNetplanRoundTrip having to have a bunch of copy/pasted definitions, we would instead define a single test method which is parameterised with `yaml` and `expected_netplan` from each of the dicts.  (Something like `@pytest.mark.parametrize("yaml,expected_netplan", [(entry["yaml"], entry["expected_netplan"]) for entry in NETWORK_CONFIGS.values()])`.)16:52
Odd_BlokeSo anything new added to NETWORK_CONFIGS will automatically be picked up by those tests (and we lose a lot of boilerplate code too).16:53
=== philroche_ is now known as philroche
minimalOdd_Bloke: nor familiar with that but it sounds good. For each test you'd need to be able to define which of multiple inputs to each test (netconfig v1, v2, eni?) and which (or all) of the outputs17:00
Odd_Blokeminimal: I think you'd define a separate test for each of the dict keys: different renderers have different setup requirements for testing.17:08
Odd_BlokeAnd then (and now, for that matter) you can run only the Netplan tests with: pytest -k TestNetplanRoundTrip tests/unittests/test_net.py17:10
minimalOdd_Bloke: BTW what's your view on Mina's comments on PR 623?17:11
Odd_Bloke(Due to our pytest support matrix (our tests need to run on the version of pytest in Ubuntu 16.04), I don't think we can mark up each of the cases to be able to select quite as specifically as "the 'small' configuration on netplan".  Given that _all_ the test_net tests currently run in ~2s, I don't think that's too painful.)17:12
smoserwarning, stupid question17:17
smosermeena: do you use systemd on freebsd ?17:17
smosersomeone have time to review https://github.com/canonical/cloud-init/pull/62217:21
meenasmoser, nope, we can't we don't have cgroups on freebsd17:29
meenahttps://www.freebsdnews.com/2018/08/21/benno-rice-the-tragedy-of-systemd-bsdcan-2018/ good talk on why freebsd could really use something like systemd17:29
meenawe also don't have the probably easier to port Solaris contracts, which are roughly the same concept17:30
Odd_Blokeminimal: Have replied on #623. :)17:37
minimalOdd_Bloke: Thanks, will be glad to put that PR to bed :-)17:39
smosermeena: so why do you care about ds-identify ?17:42
meenasmoser: because it's what guesses which cloud you're on, and on FreeBSD, it's ca 95% wrong.17:42
meenahow the heck do i get the first "word" in a $line in bash? $line[0] does not cut it18:15
meenais bash the worst programming language?18:19
meenaoh, never mind, this isn't bash, this is sh.18:26
minimalcut -f1 ?18:28
meenaminimal: that'd be 3 cut calls18:30
minimalefficiency isn't shell's selling point :-)18:30
meenai thought efficiency was the point of ds-identify18:31
meenanothing else uses cut in that code18:31
meenai'm already using awk to normalise an output18:31
meenanothing else does that either!18:32
meenathere's one call to grep18:32
meenano sed18:32
meenasmoser: ping18:38
smosermeena: 2 minutes ?18:38
smoser(i have to go, in < 2 minutes . return in 40)18:39
smosersorry. out.18:41
smosermeena: back19:19
meenasmoser: https://github.com/canonical/cloud-init/pull/62519:24
meenai'm surprised you can do so much with ds-identify without shelling out…19:25
meenawhat i'm not surprised about is how the result looks (ugly, af)19:25
smoseri've written way to much shell in my life.19:25
meenasmoser: examples of what the lines look like are in https://bugs.launchpad.net/cloud-init/+bug/190117419:32
ubot5Ubuntu bug 1901174 in cloud-init "ds-identify cannot identify disk-based resources on FreeBSD" [Undecided,New]19:32
meenasmoser: i don't understand this: "also... maybe we consider trying to render different versions ? freebsd and linux ?"19:37
smoserwe could have a ds-identify on linux that only contained linux19:51
smoserand one for freebsd that only contained freebsd19:51
smoserwas what i was suggesting.19:52
smoserand "render" one file ds-identify based on which platform was being installed for19:52
meenasmoser: i'll leave this to more advanced developers of cloud-init19:55
smoserwhat you're doing is probably ok.19:56
meenayeah, i don't get blkid not found errors any more ;)19:56
smoserbut i dont want more confusing code to read (than it already is) or slower runtimee19:56
meenai've now resolved two of your review complaints, https://github.com/canonical/cloud-init/pull/62519:57
meenaso, i have 3 pull requests in the race right now, and all of them are going to break me because of tests.20:10

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!