Bug 85267 – NRWT with --leaks shows leaks from old runs

• Component: Tools / Tests
• Status: RESOLVED DUPLICATE

[Tim Horton]

Created attachment 139589 [details]
patch

A note from webkitpy/layout_tests/port/mac.py:

# FIXME: This will include too many leaks in subsequent runs until the results directory is cleared!

This is indeed the case, and it caught me up earlier today. Seems pretty unfortunate.

I hacked up a horrible solution, which is basically just shoving a UUID through everywhere and using that in the leaks filename. I don't like it, I'm hoping dpranke or someone who knows the code better can come up with something better (I'm happy to implement it, I just need a better idea). (for one: why don't we just capture the output and fling it back through to the manager (can we go in that direction?), and have it all written out into one file per run; that way they'd all be together and not get mixed up).

[Tim Horton]

Created attachment 139595 [details]
patch

[Dirk Pranke]

Hm. I didn't write this code, so I'm cc'ing eric (it looks like this was mostly eric and adam's work). 

That said, I have a couple of thoughts. 

First, it looks like the leak files are written to the results directory, you could avoid this problem altogether by deleting any old leak files prior to the run either by running with --clobber-old-results or by doing a more specific deletion (e.g., in setup_test_run()).

Second, if you wanted to keep perhaps it's enough to look for any leak files written since the start of the test run? You could catch the start of the run in setup_test_run() or even Port.__init__() and then modify leaks_file_in_directory to take a timestamp and filter for files with a ctime (or an mtime) > that time.

I'd probably vote for option #2, above.

However, if you really wanted to go the uuid route, I would probably pass it through via the WorkerArguments class populated in manager.py and then passed to the worker in start_worker().

Then, of course, we could approach this a completely different way ...

Right now we check for leaks when we stop the process; the problem with capturing that output and passing it back to the manager is that it isn't directly correlated to a particular test, and so it doesn't fit the normal model of messaging the workers use. 

There's nothing to stop us from, for example, modifying driver.stop() to pass back an object and then sending that object back in a message to the manager, which could then aggregate the objects and write into a single file, as you suggest. It wouldn't be much different from the set_uuid() message you added, it would just go in the opposite direction.

[Dirk Pranke]

I think this was fixed with the patch in bug 84196 (r114577), right? I'm closing this; please re-open if I'm missing something.

*** This bug has been marked as a duplicate of bug 84196 ***