RESOLVED FIXED 79872
perftestsrunner can call printer.write() after printer.cleanup() 
https://bugs.webkit.org/show_bug.cgi?id=79872
Summary perftestsrunner can call printer.write() after printer.cleanup()
Dirk Pranke
Reported 2012-02-28 22:22:11 PST
perftestsrunner can call printer.write() after printer.cleanup()
Attachments
Patch (2.54 KB, patch)
2012-02-28 22:28 PST, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
don't call run() twice instead (1.78 KB, patch)
2012-02-28 22:41 PST, Dirk Pranke 		rniwa: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-02-28 22:26:24 PST
PerfTestsRunner.run() calls printer.cleanup(), and you should not call any methods on printer after cleanup(). In the unittests, we call run() twice in a row (in test_run_with_upload_json), and the second time attempts to write something. We should either not call cleanup() during run(), or reinitialize the printer during run(), or not call run() twice. I will post a patch that does the first thing, but I'm not sure if you're supposed to be able to call run() twice in a row or not.
Dirk Pranke
Comment 2 2012-02-28 22:28:34 PST
Created attachment 129391 [details] Patch
Ryosuke Niwa
Comment 3 2012-02-28 22:31:39 PST
Comment on attachment 129391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129391&action=review > Tools/ChangeLog:13 > + It was a fluke that this happened to work, but it is still a bug > + that should be fixed either by not calling cleanup() during > + run() or by not calling run() twice. This patch does the former. Can we take the latter approach instead? We shouldn't be calling run() twice.
Dirk Pranke
Comment 4 2012-02-28 22:36:10 PST
Sure, I'll rework it.
Dirk Pranke
Comment 5 2012-02-28 22:41:15 PST
Created attachment 129392 [details] don't call run() twice instead
Dirk Pranke
Comment 6 2012-02-28 22:41:45 PST
Comment on attachment 129392 [details] don't call run() twice instead please take another look?
Dirk Pranke
Comment 7 2012-02-29 11:48:46 PST
Committed r109235: <http://trac.webkit.org/changeset/109235>
Note You need to log in before you can comment on or make changes to this bug.