Ah, I was wondering what the proper way to do that was, sorry for the confusion. My go to was to respond to previous version messages as whether or not the feedback had been added. As far as default behavior for igt tests, I wouldn't be averse to it, maybe Thomas Wood can chime in on that?
-----Original Message----- From: Daniel Vetter [mailto:[email protected]] On Behalf Of Daniel Vetter Sent: Friday, August 15, 2014 4:19 AM To: Atwood, Matthew S Cc: [email protected] Subject: Re: [Piglit] [PATCH v3] programs/run.py add file sync command line option On Tue, Jan 03, 2012 at 09:08:25PM -0800, Matthew Atwood wrote: > From: Matt Atwood <[email protected]> > > Currently while running igt tests a kernel panic causes the results > json file to lose all data. This patch adds a command line option (-s, > --sync) that syncs the file descriptor to disk after every test > allowing data to persist through hard crashes. When resending patches please add a short changelog here (or below the --- line, not sure about piglit standard) so that people can see at a glance what changed. I wonder whether we shouldn't set this by default in tests/igt.py like we do with dmesg already. igt testcases are a lot slower than opengl piglit tests, so the added overhead should hurt badly. And if it does we can always add a --no-sync later on. -Daniel > --- > framework/core.py | 3 ++- > framework/programs/run.py | 14 ++++++++++---- > framework/results.py | 15 ++++++++++++++- > 3 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/framework/core.py b/framework/core.py index > d3922a9..8c4ff57 100644 > --- a/framework/core.py > +++ b/framework/core.py > @@ -100,7 +100,7 @@ class Options(object): > """ > def __init__(self, concurrent=True, execute=True, include_filter=None, > exclude_filter=None, valgrind=False, dmesg=False, > - verbose=False): > + verbose=False, sync=False): > self.concurrent = concurrent > self.execute = execute > self.filter = [re.compile(x) for x in include_filter or []] > @@ -109,6 +109,7 @@ class Options(object): > self.valgrind = valgrind > self.dmesg = dmesg > self.verbose = verbose > + self.sync = sync > # env is used to set some base environment variables that are not > going > # to change across runs, without sending them to os.environ which is > # fickle as easy to break > diff --git a/framework/programs/run.py b/framework/programs/run.py > index eb67d7f..468bff4 100644 > --- a/framework/programs/run.py > +++ b/framework/programs/run.py > @@ -97,6 +97,9 @@ def run(input_): > type=path.realpath, > metavar="<Results Path>", > help="Path to results folder") > + parser.add_argument("-s", "--sync", > + action="store_true", > + help="Sync results to disk after every test") > args = parser.parse_args(input_) > > # Disable Windows error message boxes for this and all child processes. > @@ -132,7 +135,8 @@ def run(input_): > execute=args.execute, > valgrind=args.valgrind, > dmesg=args.dmesg, > - verbose=args.verbose) > + verbose=args.verbose, > + sync=args.sync) > > # Set the platform to pass to waffle > if args.platform: > @@ -154,7 +158,7 @@ def run(input_): > # Begin json. > result_filepath = path.join(args.results_path, 'results.json') > result_file = open(result_filepath, 'w') > - json_writer = framework.results.JSONWriter(result_file) > + json_writer = framework.results.JSONWriter(result_file, > + opts.sync) > > # Create a dictionary to pass to initialize json, it needs the contents > of > # the env dictionary and profile and platform information @@ > -211,7 +215,8 @@ def resume(input_): > execute=results.options['execute'], > valgrind=results.options['valgrind'], > dmesg=results.options['dmesg'], > - verbose=results.options['verbose']) > + verbose=results.options['verbose'], > + sync=results.options['sync']) > > core.get_config(args.config_file) > > @@ -219,7 +224,8 @@ def resume(input_): > opts.env['PIGLIT_PLATFORM'] = results.options['platform'] > > results_path = path.join(args.results_path, 'results.json') > - json_writer = framework.results.JSONWriter(open(results_path, 'w+')) > + json_writer = framework.results.JSONWriter(open(results_path, 'w+'), > + opts.sync) > json_writer.initialize_json(results.options, results.name, > core.collect_system_info()) > > diff --git a/framework/results.py b/framework/results.py index > 4b5fb30..efc7029 100644 > --- a/framework/results.py > +++ b/framework/results.py > @@ -102,8 +102,9 @@ class JSONWriter(object): > > INDENT = 4 > > - def __init__(self, f): > + def __init__(self, f, fsync): > self.file = f > + self.fsync = fsync > self.__indent_level = 0 > self.__inhibit_next_indent = False > self.__encoder = json.JSONEncoder(indent=self.INDENT, > @@ -175,6 +176,12 @@ class JSONWriter(object): > self.file.close() > > @synchronized_self > + def __file_sync(self): > + if self.fsync: > + self.file.flush() > + os.fsync(self.file.fileno()) > + > + @synchronized_self > def __write_indent(self): > if self.__inhibit_next_indent: > self.__inhibit_next_indent = False @@ -201,6 +208,7 @@ > class JSONWriter(object): > self.__indent_level += 1 > self.__is_collection_empty.append(True) > self._open_containers.append('dict') > + self.__file_sync() > > @synchronized_self > def close_dict(self): > @@ -212,6 +220,7 @@ class JSONWriter(object): > self.file.write('}') > assert self._open_containers[-1] == 'dict' > self._open_containers.pop() > + self.__file_sync() > > @synchronized_self > def write_dict_item(self, key, value): > @@ -221,6 +230,8 @@ class JSONWriter(object): > # Write value. > self.__write(value) > > + self.__file_sync() > + > @synchronized_self > def write_dict_key(self, key): > # Write comma if this is not the initial item in the dict. > @@ -235,6 +246,8 @@ class JSONWriter(object): > > self.__inhibit_next_indent = True > > + self.__file_sync() > + > > class TestResult(dict): > def __init__(self, *args): > -- > 1.8.3.2 > > _______________________________________________ > Piglit mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
