storaged: Fix writing fstab when modifying NFS mounts

[martinpitt]

readlines() keeps the \n line endings, so joining the array with \n caused extra empty lines to be inserted between each existing line. Move to splitlines() instead, which strips the \n and also makes the del special case obsolete.

Make the integration test much more picky: Don't just assert the single line we expect to get added, but the full file. This will catch not only extra empty lines, but also accidental removals or unexpected ordering.

https://bugzilla.redhat.com/show_bug.cgi?id=2160256

[martinpitt]

Without the fix, the test fails like this, reproducing the bug:

Traceback (most recent call last):
  File "/var/home/martin/upstream/cockpit/realms/test/verify/check-storage-nfs", line 65, in testNfsClient
    self.assertEqual(new_fstab, orig_fstab + "127.0.0.1:/home/foo /mnt/test nfs defaults")
AssertionError: "\n\n#\n\n# /etc/fstab\n\n# Created by anaco[788 chars]ts\n" != "\n#\n# /etc/fstab\n# Created by anaconda on[756 chars]ults"
- 
  
  #
- 
  # /etc/fstab
- 
  # Created by anaconda on Sat Nov  5 08:17:27 2022
- 
  #
- 
  # Accessible filesystems, by reference, are maintained under '/dev/disk/'.
- 
  # See man pages fstab(5), findfs(8), mount(8) and/or blkid(8) for more info.
- 
  #
- 
  # After editing this file, run 'systemctl daemon-reload' to update systemd
- 
  # units generated from this file.
- 
  #
- 
  UUID=3975d5e5-e66a-44c3-90f7-dc2d1c7859d1 /                       btrfs   subvol=root,compress=zstd:1 0 0
- 
  UUID=efdb1b15-eb33-4f1d-acb8-885e29df6783 /boot                   ext4    defaults        1 2
- 
  UUID=C0A6-C7D5          /boot/efi               vfat    defaults,uid=0,gid=0,umask=077,shortname=winnt 0 2
- 
  UUID=3975d5e5-e66a-44c3-90f7-dc2d1c7859d1 /home                   btrfs   subvol=home,compress=zstd:1 0 0
- 
- 127.0.0.1:/home/foo /mnt/test nfs defaults
?                                           -
+ 127.0.0.1:/home/foo /mnt/test nfs defaults

[mvollmer]

is maxDiff used anywhere?

[mvollmer]

This line should be removed, I think.

[martinpitt]

I kept it in as otherwise you don't get a diff like in the comment above, but just a message that gives you the first few characters and a "if you want to see the full diff, set maxDiff".

I don't feel strongly about it at all, though -- I'm happy to take it out, and we can put it back in locally if we need to debug it.

[martinpitt]

Ich hab es outgetaked. 😁

[mvollmer]

Ahh, it's a unittest thing? I didn't know that, I thought it's some debugging leftovers.

So, up to you, really. But if we keep it in, it needs a comment.

(Inheritance is wonderful... :-)

[mvollmer]

So, up to you, really. But if we keep it in, it needs a comment.

Per IRC, let's not.

[mvollmer]

Looks good in general, thanks!