Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moved UI screenshots inside the repository using git-lfs #8392

Merged
merged 14 commits into from Aug 31, 2016
Merged

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Jul 21, 2015

Fixes #7726

FYI I placed the screenshots into a new directory called expected-screenshots instead of the existing expected-ui-screenshots. The reason for that is because the existing dir is a submodule, and handling conflicts/branches with such a change is very very painful. Maybe after merging we could rename the directory if anyone prefers so.

TODO:

@mnapoli mnapoli added the c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. label Jul 21, 2015
@mnapoli mnapoli added this to the Short term milestone Jul 21, 2015
@mnapoli mnapoli added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Jul 25, 2015
mnapoli added a commit to matomo-org/developer-documentation that referenced this pull request Aug 3, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 3, 2015

Current status: even though I updated the screenshots, the Travis build uses a previous version. This is very weird because the screenshots are correct (up to date) both locally and on GitHub, but the screenshots being used in the Travis build are incorrect (not up to date).

One possibility is that git lfs is somehow downloading an old version of the screenshots… The reason for that is still unknown (is it a problem with the git extension, the GitHub git-lfs server, etc…).

@mattab mattab modified the milestones: 2.15.0, Short term Aug 10, 2015
@mattab
Copy link
Member

mattab commented Aug 10, 2015

Hey @mnapoli, was there any progress or maybe you found some information what was not working? It would be so great to use git-lfs soon!

@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 10, 2015

@mattab I was waiting for a new release, but I'll try to make it work today and see if I can get around this buggy behavior.

mnapoli added a commit that referenced this pull request Aug 10, 2015
mnapoli added a commit that referenced this pull request Aug 10, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 10, 2015

I'm still trying to make the build work (git lfs installation issues now with the new version). I just discovered this other issue and we might need to wait for it to be fixed: git-lfs/git-lfs#564 (may be fixed in v0.6.0)

I'll try to have a green build (to have this PR working at least).

mnapoli added a commit that referenced this pull request Aug 10, 2015
mnapoli added a commit that referenced this pull request Aug 10, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 10, 2015

I'm still stuck at the same place: Travis downloads old versions of some screenshots… I think it's a bug with git-lfs but it could be the client or the server… (maybe git-lfs/git-lfs#564)

I would give it another try when v0.6.0 is released.

@mattab
Copy link
Member

mattab commented Aug 14, 2015

it looks like they fixed git-lfs/git-lfs#564 - which maybe would unblock us.

@mnapoli do you know if maybe they have nightly builds or beta builds that we could test already?

@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 14, 2015

@mattab had a look last time but it requires compiling the whole thing and there are no instructions IIRC. I can check again next week.

@mattab
Copy link
Member

mattab commented Aug 14, 2015

if we're lucky, compiling could be as easy as ./configure && make && make install ?

@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 14, 2015

Nope, no Makefile, they use Go and they have custom script for e.g. Debian builds.

@mattab mattab modified the milestones: 2.15.0, 2.15.1 Aug 17, 2015
@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 19, 2015

Tried with the current master (matomo-org/travis-scripts@bfb563a) but it seems images downloaded by git-lfs are not even correct images unfortunately. The build shows all image comparison failing (https://travis-ci.org/piwik/piwik/jobs/76290929) with:

Processed screenshot does not match expected for ActionsDataTable_column_sorted.png (image magick error: compare: improper image header `/home/travis/build/piwik/piwik/tests/lib/screenshot-testing/../../../tests/UI/./expected-screenshots/ActionsDataTable_column_sorted.png' @ error/png.c/ReadPNGImage/3246.)

Not looking good for now.

@tsteur
Copy link
Member

tsteur commented Oct 27, 2015

Had a rough look at this and noticed there were Error trying to create local media directory in '/home/travis/build/piwik/piwik/.git/lfs/objects/d1/5f': mkdir /home/travis/build/piwik/piwik/.git/lfs/objects/d1: permission denied errors in the output of git lfs fetch and it said (262 of 262 files) 0 B / 21.95 MB.

I fixed this and it does not output any errors anymore but says (262 of 262 files) 19.87 MB / 21.95 MB so it still seems to not fetch all data for some reasons. Maybe this bug will be fixed in the next git-lfs version and would be just wait until there is a new version available

@mattab
Copy link
Member

mattab commented Oct 27, 2015

Maybe this bug will be fixed in the next git-lfs version and would be just wait until there is a new version available

Maybe we could check if it's a known bug and if not we could report it to git-lfs team?

@tsteur
Copy link
Member

tsteur commented Oct 27, 2015

I did and there are some reports and I think there's one PR in review that might fix it but could be also totally unrelated. I'd say for now easiest would be maybe just to wait for a little

@tsteur
Copy link
Member

tsteur commented Aug 30, 2016

sounds good to me. FYI: I have merged master into 3.x just yesterday so it shouldn't be as painful :)

@sgiehl
Copy link
Member

sgiehl commented Aug 30, 2016

Ok. I'll schedule this for friday night or saturday.

@sgiehl sgiehl modified the milestones: 2.16.3, 3.0.0-b1 Aug 31, 2016
@sgiehl sgiehl merged commit d278d48 into master Aug 31, 2016
@sgiehl sgiehl removed the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Aug 31, 2016
@tsteur
Copy link
Member

tsteur commented Aug 31, 2016

FYI: When you merge master => 3.x-dev I'd recommend to directly use the screenshots from https://github.com/piwik/piwik-ui-tests/tree/3.0-m06

@sgiehl
Copy link
Member

sgiehl commented Aug 31, 2016

Already about to do that. Currently trying to resolve the merge conflicts...

@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 31, 2016

\o/ congrats!

@mattab mattab added the Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. label Oct 2, 2016
mattab added a commit that referenced this pull request Nov 9, 2016
was introduced in d57f4a8 
but deprecated with git lfs #8392
mattab added a commit that referenced this pull request Nov 14, 2016
was introduced in d57f4a8 
but deprecated with git lfs #8392
@mattab mattab deleted the git-lfs branch December 23, 2016 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants