469806 - Tests wanted to verify the sameness of extensions.blocklist.url format

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Daniel Einspanjer [:dre] [:deinspanjer]]

Bug 449027 implemented a change in the blocklist URL that incremented the "version" of it from 2 to 3.  This caused a problem with the parser that processes those requests for metrics because it wasn't updated to understand the new version.

If we could have some sort of test that just makes sure that the currently used value is equal to the string that metrics is expecting, then the test failure message could instruct the developer to log a bug against metrics detailing the change.

Comment 1

[Daniel Einspanjer [:dre] [:deinspanjer]]

If this proposal is accepted, I'll open a new bug for the Applications Update component to have the same test.

Comment 2

[Daniel Einspanjer [:dre] [:deinspanjer]]

(In reply to bug 469760 comment #18)
> (In reply to bug 469760 comment #17)
>> I just went back and looked at the regex.  It would properly handle an escaped
>> bogus value as well without a problem, but I'd still very much like to get
>> mindset that any time a change happens with the structure of one of these URLs,
>> a bug gets opened with metrics to review it for potential impact.  I've opened
>> Bug 469806 to that effect.

> I'm not entirely sure what the benefit is in opening a bug to evaluate the
> potential impact of another bug. Surely it would be better for all the
> discussion to go in the single bug?

If you take a look at bug 449027, there was nothing in the bug description or comments indicating that the format of the extensions.blocklist.url was changing, so even if I had a saved bugzilla search that looked for extensions.blocklist.url and other keywords, I wouldn't have found this bug, and obviously, no one knew to inform me of the change until we discovered that metrics had stopped reporting the nightly builds and beta 2.

Having a test that warned the developer that further action was needed seems to be the easiest way to resolve that problem.  I'm certainly open to alternatives though.

Comment 3

[Dave Townsend [:mossop]]

(In reply to comment #2)
> (In reply to bug 469760 comment #18)
> > (In reply to bug 469760 comment #17)
> >> I just went back and looked at the regex.  It would properly handle an escaped
> >> bogus value as well without a problem, but I'd still very much like to get
> >> mindset that any time a change happens with the structure of one of these URLs,
> >> a bug gets opened with metrics to review it for potential impact.  I've opened
> >> Bug 469806 to that effect.
> 
> > I'm not entirely sure what the benefit is in opening a bug to evaluate the
> > potential impact of another bug. Surely it would be better for all the
> > discussion to go in the single bug?
> 
> If you take a look at bug 449027, there was nothing in the bug description or
> comments indicating that the format of the extensions.blocklist.url was
> changing, so even if I had a saved bugzilla search that looked for
> extensions.blocklist.url and other keywords, I wouldn't have found this bug,
> and obviously, no one knew to inform me of the change until we discovered that
> metrics had stopped reporting the nightly builds and beta 2.
> 
> Having a test that warned the developer that further action was needed seems to
> be the easiest way to resolve that problem.  I'm certainly open to alternatives
> though.

I wasn't suggesting that this bug is not useful. But that opening a new bug everytime a future bug will affect the url is not useful

Comment 4

[Daniel Einspanjer [:dre] [:deinspanjer]]

Hrm.  Well, in the case of bug 449027, it would have been "good enough" to just get someone from metrics CC'ed on the bug.  I have to imagine that we probably would have ended up opening a new bug on our side of it anyway though so that we could reflect the proper product/component that our change would go into and to be able to track what code stream included the change.

I would be happy to just be CC'ed on the original bug and then I can take care of the second half on my own if you don't think the developer implementing the change should have to create the metrics bug.

Comment 5

[Robert Strong (they/them - no direct email)]

Wouldn't a comment to open a bug against metrics for the change in the prefs file that contains the url be enough? It seems like a test is overkill and is no more foolproof than a comment in the prefs file since all the dev would have to do is update the test when they update the url to get around this.

Comment 6

[Daniel Einspanjer [:dre] [:deinspanjer]]

I think a comment is a lot easier to accidentally overlook or notice and forget about than a test failure is.
If the developer purposefully changes the test and ignores the comment/documentation of the test to file a bug, well, that is just about malicious and I don't think we need to plan against that.

I don't guess this has to be a dev unit test though, maybe it could be something that has the same gravity as a l10n string change such that QA or someone will not let it just be overlooked?

I'm not trying to be picky or overbearing here.  I just want to save the time and grief we have hit a few times now where format changes have snuck up on us and ended up causing a *lot* more work than they would have if we were in the loop earlier.

Comment 7

[Robert Strong (they/them - no direct email)]

That's fine though the only experience I personally had with this is where I did notify you... perhaps we can add a test for this? ;)

If we do add a test for this it should live in browser since each app has to define this.

Comment 8

[Daniel Einspanjer [:dre] [:deinspanjer]]

if (did_developer_notify()) {
  process_pat_on_back();
} else {
  frown();
}

Comment 9

[Andy McKay]

Due to a long period of inactivity on this bug (6.36 years), I am intending to close this bug within a month or so in accordance with: https://wiki.mozilla.org/Add-ons/OldBugs Please remove [intent-to-close] from the whiteboard and comment on this bug if you would like to keep it open.