search

krisselden / ember-macro-benchmark

Benchmark recording of an ember app with running with 2 versions of Ember.
25 stars 9 forks source link

Don't pass fingerprint to assets on disk #29

Closed mixonic closed 7 years ago

mixonic commented 7 years ago

The fingerprint changes in https://github.com/krisselden/ember-macro-benchmark/pull/26 seem incorrect. The asset fingerprint from the request should not be looked for on disk.

/cc @rondale-sc

rondale-sc commented 7 years ago

@mixonic I believe that is correct. Robert and I mentioned it, but likely there will be rework around the quest issue so we haven't addressed it yet. I belive fingerprint lookup on disk is fine so long as the har file is generated by running the server with those fingerprinted assets, but that kinda defeats the purpose.

I've been disabling fingerprinting with production builds to do comparisons for my project. We should definitely chat about putting together a more detailed outline of how fingerprints should be handled. Ideally we could fuzzy match on the filename and ignore the fingerprint altogether.

mixonic commented 7 years ago

I've been disabling fingerprinting with production builds to do comparisons for my project.

Yeah that is what I was doing as well, but without this patch the un-fingerprinted files are never matched. The fingerprint from the HTTP request is looked up and the file on disk is not found. The server falls back to the HAR. I'm not sure how you can use this tool at all without this fix.

mixonic commented 7 years ago

Sorry, you are right that you can use this tool without the fix this way: Create a new HAR for every build. But yes, that is not the point.

krisselden commented 7 years ago

The point though is if you vary the dist you want to control the recording. @mixonic is correct we want to ignore the fingerprint altogether.

krisselden commented 7 years ago

Ideally the dist you test would be not fingerprinted

mixonic commented 7 years ago

@krisselden FYI I don't have merge here, or I would merge this ;-)