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

Include scheduled reports in integration tests #3323

Closed
julienmoumne opened this issue Aug 15, 2012 · 26 comments
Closed

Include scheduled reports in integration tests #3323

julienmoumne opened this issue Aug 15, 2012 · 26 comments
Assignees
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone

Comments

@julienmoumne
Copy link
Member

Scheduled reports (PDF, HTML, SMS) are currently not part of the integration test suite.

Questions to address :

  • which format should be tested, in particular, PDF might be trickier (see attached example files)
  • whether graphs (images) should be included
  • which information should be removed, e.g: dates
  • which period(s) should be tested
  • I have attached a patch for OneVisitorTwoVisitsTest.php, how can it be applied to all integration tests with minimum code duplication ?
@julienmoumne
Copy link
Member Author

@julienmoumne
Copy link
Member Author

@julienmoumne
Copy link
Member Author

@julienmoumne
Copy link
Member Author

@mattab
Copy link
Member

mattab commented Aug 17, 2012

Feedback

  • The attached html/pdf look good!
  • I don't think we should run the pdf/html tests on ALL tests. This would be too slow and not so useful. I think the main goal is to check that the basic use cases (ie. a report with tables, graphs, icons, etc.) work.
  • Graphs should be included... since they are not currently tested we could do "une pierre 2 coups". Maybe only put graphs in the PDF only to keep it simple?
  • // TODO this maybe need to be updated after adding OUTPUT_RETURN
    It looks good like it, since we want to compare the output including the graphs inlined in the doc.

Thanks for looking into it: it always made me uneasy that HTML/PDF reports were not tested. Now that we also have SMS reports, and that the 2 plugins are loosely coupled, it is very good improvement to our QA to add these tests!

@mattab
Copy link
Member

mattab commented Aug 17, 2012

which information should be removed, e.g: dates

Dates should stay since they won't/shouldn't change from run to run.

which period(s) should be tested
I'd say the period with the most data so that code coverage is maximum. For the test OneVisitorTwoVisits visits are on the same day so period=day is good enough.

@julienmoumne
Copy link
Member Author

(In [6849]) refs #3323 #3088 #2708 #71 #2318

  • generate and compare HTML, PDF & SMS reports in Test_Piwik_Integration_EcommerceOrderWithItems & Test_Piwik_Integration_TwoVisitors_TwoWebsites_DifferentDays
  • report content as return value of PDFReports->generateReport() with new output type OUTPUT_RETURN

@julienmoumne
Copy link
Member Author

(In [6883]) refs #3323

  • integration test files can now have custom file extensions (e.g: .pdf, .html, .sms.txt)

  • removing hard coded list of reports

    TODO: expected integration files need to be updated, pending GD discrepancy issues

@julienmoumne
Copy link
Member Author

(In [6932]) refs #3323 - include images in scheduled reports only if system under test matches some technical characteristics of the Piwik QA Server

TODO

  • update method 'canImagesBeIncludedInScheduledReports' to match the new Piwik QA server when it will be up and running
  • build a vagrant VM with QA server specs, generate reports using it and override expected files

@sgiehl
Copy link
Member

sgiehl commented Sep 7, 2012

Is there a reason why you made setUpScheduledReports non-static and moved the call to a seperate test case? I guess it was more correct before, as reports should be setup before running the tests (it's more part of the setup than of a test case). Btw: now it won't be possible only to run the api tests

@julienmoumne
Copy link
Member Author

(In [6936]) refs #3323 follow best practices per comment:6:ticket:3323

@julienmoumne
Copy link
Member Author

Do tell me if it's better like that. The idea is to warn the developer that some tests are being skipped because of system incompatibilities.

@julienmoumne
Copy link
Member Author

(In [6946]) refs #3323 ignoring *.html, *.pdf and *.txt files in processed directory

@mattab
Copy link
Member

mattab commented Nov 1, 2012

Looks good like that. I'm wondering if we should really test for images at all now, since most developers won't have images generated, but it could be helpful.. Can the ticket be closed?

@julienmoumne
Copy link
Member Author

static png graphs will be tested on the integration server
this insures no regression will occur, especially since we are likely to refactor underlying libraries (pChart)

@julienmoumne
Copy link
Member Author

(In [7557]) refs #3323 updating qa server specs for inclusion of static images in integration testing

@mattab
Copy link
Member

mattab commented Dec 12, 2012

(In [7604]) Refs #3323 Scheduled Reports integration tests until they are generate in expected/ with the static images same features as Jenkins build

@julienmoumne
Copy link
Member Author

(In [7613]) refs #3323

  • include back scheduled reports in integration tests
  • add readme section

@mattab
Copy link
Member

mattab commented Dec 20, 2012

(In [7675]) refs #3323 some ubuntu precise simply output ubuntu, lets try to widen the boxes that can run static images in tests

PS: all pdf tests on my box!

@anonymous-matomo-user
Copy link

In f387a89: refs #3323 enable static png images in integration tests across additional php versions

@anonymous-matomo-user
Copy link

In 824eced: refs #3323 remove unused expected integration files

@anonymous-matomo-user
Copy link

In 545c999: refs #3323 enable static png images in integration tests for travis-ci

@anonymous-matomo-user
Copy link

In efa5344: refs #3323 exclude static png graph tests in travis-ci until I can set-up travis-ci gd version on my box

@mattab
Copy link
Member

mattab commented Apr 10, 2013

thanks, travis is now green again :)

It will be good to use gd 2.0.34: I just realized most servers in productions are using this version.

Julien, after talking to Fabian, I think we should use phpenv on our dev boxes. Would you like to give it a try? I will also try to install it but if you do it first, thanks for posting here any instructions for ubuntu (which we can then add in the tests/README).

@anonymous-matomo-user
Copy link

In 18974f8: refs #3323 doc

@anonymous-matomo-user
Copy link

In b993d27: refs #3323 test for one row evolution based png graph

@julienmoumne julienmoumne added this to the 1.9.2 - Piwik 1.9.2 milestone Jul 8, 2014
@julienmoumne julienmoumne self-assigned this Jul 8, 2014
@mattab mattab added the c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. label Oct 12, 2014
This issue was closed.
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. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

4 participants