@andrzejewsky opened this issue on March 8th 2016

Recently, I made small PR: https://github.com/piwik/piwik/pull/9863 - it was merged without any tests. I'd like to improve it eg. in order to detect some regression in the future, thus I created this PR.

What do you think about it? @piwik/core-team?

Furthermore, here is PR for updating screenshots: https://github.com/piwik/piwik-ui-tests/pull/6 Look at the visitor log at the last entry: https://github.com/piwik/piwik-ui-tests/blob/updateScreenshots/UIIntegrationTest_visitors_with_site_search_visitorlog.png (before, it was displayed incorrectly)

@mattab commented on March 8th 2016

Hi @andrzejewsky

I'm not sure but maybe you have directly edited the DB dump to include the new test case?

The Omnifixture is generated by dumping the DB after importing all existing Fixtures in Piwik. When you want to modify the Omnifixture you need to: - Add or edit an existing Fixture - Regenerate the omnifixture as per this procedure: https://github.com/piwik/piwik/tree/master/tests#fixtures-for-system-tests

Let me know what you get!

@andrzejewsky commented on March 9th 2016

Ok @mattab I created SQL dump according to your suggestions. As you see there is a new fixture with site search string (which was displayed incorrectly)

@tsteur commented on March 9th 2016

BTW in case you're keen to do it: Feel free to change OmniFixture-dump.sql.gz to OmniFixture-dump.sql. For this just unpack the gz file, remove the gz here: https://github.com/piwik/piwik/blob/2.16.1-b1/tests/PHPUnit/Fixtures/UITestFixture.php#L36 . This should work and this way we get a diff for the changed OmniFixture in the future. There's no reason for it being a gz file apart from trying to save space. However, space is not such a big issue I would say and the file isn't changing so much.

@mattab commented on March 14th 2016

@andrzejewsky thanks for the PR. Could you next: - Include the submodule for UI tests which point to the git commit with the new screenshot test? - Squash the commits into one - Edit the README and remove cat OmniFixture-dump.sql | gzip > tests/resources/OmniFixture-dump.sql.gz and edit https://github.com/piwik/piwik/blob/master/tests/resources/OmniFixture-dump.README.md and remove .gz

then we will merge :+1:

@mattab commented on April 7th 2016

@andrzejewsky I can't merge the PR I think due to the submodule conflict. Maybe you could leave the submodule as it is on master branch, ie. remove it from this PR? (I'll make sure to make the build green afterwards and update the submodule).

This issue was closed on April 7th 2016
Powered by GitHub Issue Mirror