fix(test): removed favicon, was breaking re-init unit test

[Brocco]

fixes #75

Apparently the binary nature of the favicon.ico file was causing an issue when templates were being processed.

[cironunes]

Thanks for the PR. This should fix #75

[Brocco]

This is also fixed in PR #80

[cironunes]

@IgorMinar is it safe to just remove the favicon.ico or do you think that we'll need to fix the test to live with the favicon?

[IgorMinar]

we should keep the favicon because otherwise everyone we'll see annoying http 404 errors in devtools. let's fix the tests

[Brocco]

@IgorMinar by fixing the tests, I believe this will be a fix in the ember-cli repo. I have to trace back through but I believe it is choking when trying to do a file compare of the binary files. The test that is failing is when it is attempting to re-init.

[IgorMinar]

so how about we merge this in to make the build green again, file a separate issue in this repo to readd the favicon and fix the actual issue in ember-cli? once the issue is fixed there we can readd the icon back.

[cironunes]

@IgorMinar good one.

[Brocco]

@IgorMinar I think that's a good compromise for now. There is a related issue already created for the ember-cli (ember-cli/ember-cli#4876)

[IgorMinar]

Could we delete the icon in from our test just before reinit?
On Mon, Nov 30, 2015 at 5:25 PM Mike Brocchi [email protected]
wrote:

@IgorMinar https://github.com/IgorMinar I think that's a good
compromise for now. There is a related issue already created for the
ember-cli (ember-cli/ember-cli#4876
ember-cli/ember-cli#4876)

—
Reply to this email directly or view it on GitHub
#79 (comment).

[Brocco]

@IgorMinar good call on "deleting", I was able to utilize the Blueprint.ignoredFiles array to ignore the favicon file for the unit test. Branch is updated with that change.

[cironunes]

Nice! I think that now we are good to merge it and have the ci green.

[IgorMinar]

Can you add a comment there explaining what we are doing with the favicon?
Just link back to this PR.

Otherwise lgtm
On Tue, Dec 1, 2015 at 5:54 AM Ciro Nunes [email protected] wrote:

Nice! I think that now we are good to merge it and have the ci green.

—
Reply to this email directly or view it on GitHub
#79 (comment).

[cironunes]

Added the comment as @IgorMinar requested and landed as 87bfa14.

Thanks for the PR, @Brocco!

[Brocco]

@cironunes glad to assist

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}