| Summary: | nrwt: run with no args on lion, is putting baselines in platform/mac-lion, not platform/mac | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||||
| Component: | Platform | Assignee: | Dirk Pranke <dpranke> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, adele, ap, darin, eric, ggaren, jberlin, mjs, mrowe, ojan, sam, thorton | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Dirk Pranke
2012-03-13 13:20:42 PDT
We do not intend to put any results in mac-lion by hand until Mountain Lion ships. (In reply to comment #2) > We do not intend to put any results in mac-lion by hand until Mountain Lion ships. Does that mean that you don't need there to be a mac-lion directory at all? The last time I discussed this (w/ Darin, I believe), I thought there was a need for apple employees to be able to distinguish between 'mac-future' and 'mac-lion' for some period of time prior to Mountain Lion actually shipping (technically, it was Lion at the time). It may have been that the only thing in mac-lion should be a Skipped file? I think the most recent discussion of this was in https://bugs.webkit.org/show_bug.cgi?id=72748 . cf. also http://trac.webkit.org/wiki/LayoutTestsSearchPath ... If there will not be any baselines in mac-lion until ML ships, that simplifies the code slightly (mac-lion doesn't need to be in the baseline path for reads or writes, not just writes), which is why I am asking ... Created attachment 131757 [details]
Patch
ap, darin: note the tests in test_skipped_file_search_paths() and test_baseline_search_path() and make sure that they are what you'd like the behavior to be (hopefully the tests are close enough to english that their intent will be clear). If they aren't, let me know and I'll be happy to change them. In English, my expectations are: by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac 'mac-future' will only read from 'mac' and will write to 'mac'. Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines. > In English, my expectations are: > > by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac This looks right to me. > 'mac-future' will only read from 'mac' and will write to 'mac'. > > Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines. I don't know what mac-future platform is needed for, but this behavior sounds safe. Comment on attachment 131757 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131757&action=review > Tools/Scripts/webkitpy/layout_tests/port/mac.py:56 > + if self.name() == 'mac-lion': > + # mac-lion should normally write baselines into platform/mac until Mountain Lion ships. > + # It can still read baselines from platform/mac-lion, though. Can we avoid hard-coding Lion / Mountain Lion in here? This will always be the case for the currently-shipping version of the OS. (In reply to comment #8) > (From update of attachment 131757 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=131757&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/mac.py:56 > > + if self.name() == 'mac-lion': > > + # mac-lion should normally write baselines into platform/mac until Mountain Lion ships. > > + # It can still read baselines from platform/mac-lion, though. > > Can we avoid hard-coding Lion / Mountain Lion in here? This will always be the case for the currently-shipping version of the OS. I can certainly delete the "Mountain Lion" reference, and rework this so that we have a mac.CURRENT_VERSION='lion' or something - is that what you were suggesting? Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that. (In reply to comment #7) > > In English, my expectations are: > > > > by default, on lion (platform mac-lion), we will look for baselines in mac-lion and then mac, but write new baselines into mac > > This looks right to me. > > > 'mac-future' will only read from 'mac' and will write to 'mac'. > > > > Nothing should ever be written to 'platform/mac-future'. There is no 'platform/mac-future/Skipped', and there are no baselines. > > I don't know what mac-future platform is needed for, but this behavior sounds safe. 'mac-future' as a name/concept is needed for you guys to be able to run on unreleased versions of the operating system and have that get handled properly (distinguishing it from the currently shipping version). There isn't supposed to be a directory -- there is one currently checked in, but that's the result of a bug. Partially I am looking to confirm that the unreleased version of the OS doesn't need a Skipped file (at least not one that's checked into webkit). (In reply to comment #10) > Partially I am looking to confirm that the unreleased version of the OS doesn't need a Skipped file (at least not one that's checked into webkit). That's correct. (In reply to comment #9) > > Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that. MacPort has a list of OS X versions in order of release. It seems relatively easy to derive the desired information from there. (In reply to comment #12) > (In reply to comment #9) > > > > Or were you suggesting that somehow I attempt to figure out what "the currently shipping version of the OS" is dynamically? I don't know how I'd do that. > > MacPort has a list of OS X versions in order of release. It seems relatively easy to derive the desired information from there. Sure, that works as well. Created attachment 131771 [details]
don't hardcode mac-lion
Committed r110722: <http://trac.webkit.org/changeset/110722> |