On Wed, Dec 16, 2020 at 19:37:48 -0600, Ryan Gahagan wrote:
> We addressed the feedback from our previous RFC patch for the most part.
> Under src/util/virstoragefile.c, we left a cast to an integer pointer
> that Peter mentioned because we were unable to provide a better
Casting uid_t * to int * will work in this instance but is not
acceptable. The problem with casting pointers is that if sizeof(uid_t)
!= sizeof(int) casting a pointer could result into accessing invalid
memory, while if you just assign the value of integers of distinct sizes
you get an overflow at worst. That must be changed in the code.
> solution. We've written some tests for our code but our testing
> environment is not working locally (meson doesn't even recognize the
> project as a meson build project) and so we can't regenerate output or
> test our tests.
Could you elaborate? As in, post exact steps and output what you've done
and what's broken.
> It's probably bad practice but the only solution we could think of that
> would allow us to check our tests was just to email you what we've got.
> Sorry if it's not up to standard, but please let us know if there's a
> better way to do it or if you spot any problems in these tests.
It's better to commit what you have and ask directly, ideally including
output you are (not) getting.
> Under tests/qemuxml2xmltest.c:
> DO_TEST_CAPS_LATEST("disk-network-nfs");
>
> The same line would be in tests/qemuxml2argvtest.c
>
> We created the file tests/qemuxml2argvdata/disk-network-nfs.xml:
> <domain type='qemu'>
> <name>QEMUGuest1</name>
> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> <memory unit='KiB'>219136</memory>
> <currentMemory unit='KiB'>219136</currentMemory>
> <vcpu placement='static'>1</vcpu>
> <os>
> <type arch='i686' machine='pc'>hvm</type>
> <boot dev='hd'/>
> </os>
> <clock offset='utc'/>
> <on_poweroff>destroy</on_poweroff>
> <on_reboot>restart</on_reboot>
> <on_crash>destroy</on_crash>
> <devices>
> <emulator>/usr/bin/qemu-system-x86_64</emulator>
> <disk type='network' device='disk'>
> <driver name='qemu' type='raw' cache='none'/>
> <source protocol='nfs' name='/foo/bar/baz'>
> <host name='example.com' port='2049'/>
> <nfs user='nfs-user' group='nfs-group'/>
> </source>
> <target dev='vda' bus='virtio'/>
> <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
> </disk>
> <controller type='usb' index='0'/>
> <controller type='pci' index='0' model='pci-root'/>
> <input type='mouse' bus='ps2'/>
> <input type='keyboard' bus='ps2'/>
> <memballoon model='none'/>
> </devices>
> </domain>
>
> We have under tests/qemublocktest.c:
> TEST_JSON_FORMAT_NET(“<source protocol=’nfs’ name=’/foo/bar/baz’>\n”
> “ <host name=’example.com’ port=’2049’/>\n”
> “ <nfs user=’USER’ group=’GROUP’/>\n”
> "</source>\n”);
> and
> TEST_IMAGE_CREATE(“network-nfs-qcow2”, NULL);
>
> And finally under tests/virstoragetest.c:
> TEST_BACKING_PARSE(“json:{\”file\”:{\”driver\”:\”nfs\”,”
> “\”user\”:\”USER\”,”
> “\”group\”:\”GROUP\”,”
> “\”server\”: { \”host\”:\”example.com\”,”
> “\”port\”:\”2049\””
> ”}”
> “}”
> “}”,
> “<source protocol=’nfs’ name=’/foo/bar/baz’>\n”
> “ <host name=’example.com’ port=’2049’/>\n”
> “ <nfs user=’USER’ group=’GROUP’/>\n”
> “</source>\n”);
>
> Again, sorry if this looks awful. Please let us know if there's a more
> practical way to do this because submitting a commit with these tests
> would guarantee that the tests fail and the commit wouldn't be mergeable
> due to our environment issues, or if you see anything wrong with these
> tests.
Note that patches without tests are not acceptable either. Additionally
I'll not be copying your changes to appropriate places. Please add the
test code to appropriate places.
Also note that the upstream test-suite run in the CI does actually
provide the expected output. Obviously you can't use the ENV variable to
automatically overwrite your files, but you certainly can copy out the
diffs from the CI. Just commit empty files in place for the output files
and the CI will complain that they differ including the full difference.
If you don't post what's wrong I will not be guessing, so if you want
any input from us, be sure to provide as much information as you have
including exact steps.
