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

[Bug] Failing "Matomo Test / UI (1)" - Date format must be: YYYY-MM-DD - given: compareDates%5B%5D=2011-31-12 #21755

Closed
4 tasks done
rr-it opened this issue Jan 3, 2024 · 4 comments · Fixed by #21779
Closed
4 tasks done
Labels
c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone

Comments

@rr-it
Copy link
Contributor

rr-it commented Jan 3, 2024

What happened?

New PR #20718 but "Matomo Test / UI (1)" failed in Github CI:

Failing query:
date=2012-01-01&module=API&format=json&method=API.getProcessedReport&apiModule=VisitsSummary&apiAction=get&compare=1&comparePeriods%5B%5D=day&compareDates%5B%5D=2011-31-12&format_metrics=1&segment=&idSite=1&period=day

What should happen?

Date format must be: YYYY-MM-DD
So compareDates%5B%5D=2011-31-12 is invalid and instead must be compareDates%5B%5D=2011-12-31

How can this be reproduced?

Run "Matomo Test / UI (1)"

Matomo version

5.x-dev

PHP version

No response

Server operating system

No response

What browsers are you seeing the problem on?

No response

Computer operating system

No response

Relevant log output

2024-01-03T15:35:15.6983443Z ERROR API[2024-01-03 15:26:46 UTC] [4339c] Uncaught exception in API: /home/runner/work/matomo/matomo/matomo/core/Date.php(1142): Date format must be: YYYY-MM-DD, or 'today' or 'yesterday' or any keyword supported by the strtotime function (see http://php.net/strtotime for more information): false [Query: ?date=2012-01-01&module=API&format=json&method=API.getProcessedReport&apiModule=VisitsSummary&apiAction=get&compare=1&comparePeriods%5B%5D=day&compareDates%5B%5D=2011-31-12&format_metrics=1&segment=&idSite=1&period=day, CLI mode: 0]

Validations

@rr-it rr-it added Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. To Triage An issue awaiting triage by a Matomo core team member labels Jan 3, 2024
@sgiehl
Copy link
Member

sgiehl commented Jan 3, 2024

Hi @rr-it
I'm not sure which failing test your are referring to. At least I can't see one in that test suite.
Anyway. I search the code base and seems the invalid date is provided here:
https://github.com/matomo-org/matomo/blob/5.x-dev/tests/UI/specs/PeriodSelector_spec.js#L143
Not sure if this is done on purpose or was added by mistake.

@sgiehl sgiehl added c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. To Triage An issue awaiting triage by a Matomo core team member labels Jan 3, 2024
@rr-it
Copy link
Contributor Author

rr-it commented Jan 3, 2024

I found the ERROR API log-entry in the "raw logs":
grafik


This new test was introduced with #21242
And "Matomo Test / UI (1)" was also failing on this PR - but logs are no longer available.

https://github.com/matomo-org/matomo/blob/85948b63b2a2866b18a47e7d3628fe74cb0ecd46/tests/UI/specs/PeriodSelector_spec.js#L142C1-L147

        it('should select "previous period" from URL', async function () {
          await page.goto(url + '&comparePeriods[]=day&comparePeriodType=previousPeriod&compareDates[]=2011-31-12');
          await page.waitForNetworkIdle();

          expect(await getSelectedPeriodType()).to.match(/Period/);
        });

@caddoo May we change this test to compareDates[]=2011-12-31 instead of the current compareDates[]=2011-31-12?

@sgiehl
Copy link
Member

sgiehl commented Jan 4, 2024

@rr-it The test itself is passing. And the log entry actually is kind of expected in that case.
@mneudert Did you add that invalid date in the test on purpose?

@mneudert
Copy link
Member

mneudert commented Jan 8, 2024

Nope, not intended to break this way. Created a small PR to change the date as already suggested.

@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 5, 2024
@sgiehl sgiehl added this to the 5.0.2 milestone Feb 5, 2024
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. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants