Re: Repository layout

On Jun 5, 2012, at 7:06 AM, "Robin Berjon" <robin@berjon.com> wrote:

> On Jun 5, 2012, at 10:26 , Aryeh Gregor wrote:
>> On Mon, Jun 4, 2012 at 7:35 PM, Robin Berjon <robin@berjon.com> wrote:
>>> On Jun 4, 2012, at 18:14 , James Graham wrote:
>>>> Yes, but it can *also* mean that you break the test, if you aren't very careful. Consider a test that does document.getElementsByTagName("link")[0], for example.
>>> 
>>> While that is indeed within the realm of possible things, I would like to point out that it is a somewhat contrived example, and easy to fix at that. You would be breaking a broken test.
>> 
>> Not if the test is actually testing getElementsByTagName.
> 
> Not if the test is testing gTBTN *with* the link element. Or if testing the link element. Or if counting the total number of elements in the head or document. I can think of a few more cases, but they're even more contrived :)

Actually there are several more reasons why you wouldn't want a tool touching your test file, tests that have deliberately broken markup to test parser error handling, for example. 

> 
> I can't avoid noting that we are *already* using (and recommending) metadata inside tests. This is just adding one extra item.

Yes, in my experience, any time you separate data, the odds of that data getting out of sync rise exponentially. 

While there will be exceptions, the vast majority of tests will survive reserialization just fine. Keeping the metadata inside the test should be our default, only doing otherwise when explicitly necessary.

> 
>> Better to avoid it and keep the metadata separate.
> 
> Note that sidecar metadata containers are entirely possible (and, I believe, already supported) for the tinuscule number of tests that would be affected by this usage. We need those for tests that can't embed metadata as easily for other reasons anyway.

Yes, there is a partial implementation of sidecar metadata support in the build code. When present, a file in the same directory with the extension '.meta' contains the metadata and overrides any metadata that may be in the test.

> 
>> This is leaving aside the fact that it's quite nontrivial to
>> automatically inject an HTML tag in a fashion that's guaranteed to
>> parse correctly but also doesn't reserialize the file (which will
>> create lots of spurious diffs).
> 
> Hmmm, I wasn't thinking of doing this automatically — I was thinking about it being the way to flag a test as broken: add the bug link to it.

The build code already injects and updates metadata for several reasons. For example, injecting <link rel='match' ...> for reference files added by manifest, and updating the paths to the reference files when tests get re-arranged in the build output. Soon the build code will also be altering paths to support files (img src and the like) inside the test itself. 
> 


Note that we always reserialize tests (unless the test has a flag telling the build code not to) so that when the tools do update files, the only diffs are the changes made by the tools. 

When the build code does modify a test, it does it via the DOM.

Also note that when the framework imports tests, it can perform its own diffs of a normalized version of the test (ignoring metadata not relevant to the functioning of the test) against the previously imported version. This allows results entered against the previous version to still apply if the only
diffs are non-functional metadata. 

Peter

Received on Tuesday, 5 June 2012 14:39:32 UTC