On 01/01/2017 02:48 PM, Stefan Bruens wrote:
On Montag, 12. Dezember 2016 11:04:34 CET you wrote:
On 12/04/2016 05:52 PM, Stefan Brüns wrote:
From: Stefan Brüns <[email protected]>
The following checks are currently implemented:
1. listing a directory
2. verifying size of a file
3. veryfying md5sum for a file region
4. reading the beginning of a file
2) Doesn't mounting/unmounting require root? Nothing else in test/py
does. It'd be useful to put a comment at the top of the file detailing
which command one might want to add to /etc/sudoers to avoid having to
run the whole thing as root, and use sudo within the test where necessary.
It already uses sudo, the run command is visible from the log. Currently, I
just run "sudo true" prior to executing the test, the password is cached for 5
minutes.
I don't see sudo used anywhere in test/py; can you point out where you
see it using sudo at present?
+class FsImage:
...
+ def mount(self, log):
+ if not os.path.exists(self.mountpath):
+ os.mkdir(self.mountpath)
Use os.makedirs(path) instead; you can avoid the if statement, and it'll
handle parent directories too.
exist_ok only exists in python >= 3.2.
There's no need to use exist_ok; do this (example from conftest.py):
try:
os.makedirs(path)
except OSError as exc:
if exc.errno == errno.EEXIST and os.path.isdir(path):
pass
else:
raise
[email protected](scope='module', params=['fat', 'ext4'])
+def fsimage(prereq_commands, u_boot_config, u_boot_log, request):
+ """Filesystem image instance."""
+ datadir = u_boot_config.result_dir + '/'
Wouldn't it be better to put this into
u_boot_config.persistent_data_dir, plus avoid creating the image file if
it already exists? See u_boot_utils.py's PersistentRandomFile() as an
example. I wonder if that could be expanded to create files not just of
size n, but with sparse layout specs like this test uses?
See above. I don't think PersistentRandomFile() does fit here, as the files
are created *inside* an image. Maybe it could be used after the image and
filesystem is created and mounted ...
You missed my point. PersistentRandomFile() is existing code that
creates a persistent data file. You can create a new function/... that
uses the same techniques to create the data file, but make it create a
filesystem image rather than random data.
+ fstype = request.param
+ imagepath = datadir + '3GB.' + fstype + '.img'
+ mountpath = datadir + 'mnt_' + fstype
+
+ with u_boot_log.section('Create image "{0}"'.format(imagepath)):
+ fsimage = FsImage(fstype, imagepath, mountpath)
+ fsimage.mkfs(u_boot_log)
+
+ yield fsimage
+ fsimage.unmount(u_boot_log)
Unmounting seems to happen in a lot of different places. Can we isolate
it to just one place?
The image is mounted/unmounted for two different reasons - creating/populating
the image, and when accessing it using the hostfs commands.
IIRC, there's more duplication than that, but I'll look again when this
is reposted.
Also, what happens if the code throws an exception after obtaining an
fsimage from this generator; I'm not sure that any cleanup happens in
that case. Should there be "try: ... finally: unmount()" somewhere to
clean up even in the case of an error? Alternatively, perhaps class
FsImage should have a destructor that does the unmount (at least if it
hasn't happened already)?
Its a fixture and will be torn down by pytest.
How does that work? Once the fixture function has yielded the image,
surely if an exception is thrown, it'll be "thrown through" the
generator function and hence prevent the rest of the function body from
running? Or is there some special magic that lets the generator
complete, even if the yield effectively threw an exception?
+ return output[1:3]
No error checking for output[0]? I suppose if u_boot_console_base.py's
bad_pattern_defs[] included error patterns that "readcmd" was expected
to emit, that'd be fine, but it doesn't currently. Maybe we expect that
the other command can't possibly succeed if the read doesn't. Similar
comment for run_sizecmd() below, and perhaps elsewhere.
filesize is only set if the read succeeds, and "env print filesize" matches
bad_pattern_defs in case of an error.
What pattern is printed that matches bad_pattern_defs? Nothing in
bad_pattern_defs obviously would get printed if $filesize wasn't set.
Also, what if $filesize is still set from an earlier test (e.g. in a
different Python file), and a later test fails, yet this isn't detected
since that relies on $filesize not being set? I see this script doing
"env set filesize" /after/ its own tests which should prevent this, but
not /before/ the first test.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot