Found this while doing manual QA on bug 981842. An active experiment doesn't have the right ID when it's in the Experiments._experiments map. From the browser console, I ran this snippet: Experiments.instance().getExperiments().then(r=>console.log(r.toSource())) "[{id:(void 0), name:"Readable and Closable Tabs", description:"Customize the size and close buttons on Firefox tabs", active:true, endDate:1395282798778, detailURL:"https://addons.mozilla.org/en-US/firefox/addon/readable-and-closable-tabs/?src=api"}]" I reproduced this using http://benjamin.smedbergs.us/telex-bug985670-2/manifest/Firefox/30 as the manifest URI, but I'm not 100% sure whether this requires running the "broken" manifest first: http://benjamin.smedbergs.us/telex-bug985670/manifest/Firefox/3 I'll check in a bit.

Additional testing: On a clean profile, when I first load from HTTP the ID is correct. When I quit and reload, the ID is undefined. So the bug is somewhere in _populateFromCache. We're setting the map entry using .set(item.id, entry) From the log: 1395264393759 Browser.Experiments TRACE Experiments::populateFromCache() - data: {"version":1,"data":[{"_enabled":true,"_manifestData":{"startTime":0,"xpiHash":"sha256:e661508263fd1f949481cc79408a9806c0dade27225fcf5e6c211861fd5e7656","maxActiveSeconds":20160,"xpiURL":"http://benjamin.smedbergs.us/telex-bug985670/experiment.xpi","endTime":2234567890,"appName":["Firefox"],"id":"readable-closable-tabs@benjamin.smedbergs.us","channel":["default"]},"_needsUpdate":false,"_randomValue":0.037506727264044804,"_failedStart":false,"_name":"Readable and Closable Tabs","_description":"Customize the size and close buttons on Firefox tabs","_homepageURL":"https://addons.mozilla.org/en-US/firefox/addon/readable-and-closable-tabs/?src=api","_startDate":1395264274776,"_endDate":null}]} So the problem is that the ID is in _manifestData.id not just .id. But I think we should use entry.id instead of item.id anyway.

Do the tests already read from the cache somewhere or do I need to add a new test for that?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Do the tests already read from the cache somewhere or do I need to add a new > test for that? We do have the cache being used, but we don't have coverage for it restoring the state properly. We could easily add this in test_api.js, e.g. by: * not removing the cache file at the end of test_getExperiments() * add a task after it that creates a new Experiment * assert the state is restored as expected in the previous task * remove the cache file etc.

The test coverage will be handled by bug 987207.