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

Fix failing UserId tests #18748

Merged
merged 1 commit into from Feb 7, 2022
Merged

Fix failing UserId tests #18748

merged 1 commit into from Feb 7, 2022

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Feb 7, 2022

Description:

The UserId tests were actually failing due to this change: #18636

The reason for that seems to be the weird tracking done for the UserId tests, causing the tracked data to be slightly different.
I'll add some comments in the code to make that a bit more clear what the issue was.

refs #18082

Review

@sgiehl sgiehl added 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. Needs Review PRs that need a code review labels Feb 7, 2022
@sgiehl sgiehl added this to the 4.8.0 milestone Feb 7, 2022
Comment on lines -41 to -46
if ($numVisits % 5 == 0) {
$t->doTrackSiteSearch('some search term');
}
if ($numVisits % 4 == 0) {
$t->doTrackEvent('Event action', 'event cat');
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing the sitesearch and event tracking upfront actually caused the problems. At that point the forced date time was not yet set. So the date time from the previous iteration was used. For the first iteration of a userid that means it used the date time from the last iteration from the previous user id.

So the first iteration of userid3 inserted a visit for e.g. '2010-02-20' for the event tracking and sitesearch, but the pageview below was than inserted for '2010-02-01'. Each iteration then forces a new visit to be inserted. So a couple iterations later a second visit for '2010-02-20' with the exact same time will be inserted. The next iteration then tracks the sitesearch and event. To map those actions to the correct visit the tracker tries to load the visit to increase the action counters. But there are now two visits with the exact same parameters (idvisitor, user_id, visit_last_action_time, ...), so it's actually up to the database to decide which of the records to return first. That behavior changed due to the new index, as that seems to have changed the order in which the records are returned.

As this is actually only a very test specific issue, that won't occur while normal tracking, I've adjusted the tracking to be a bit more realistic and avoid duplicate visit entries...

Copy link
Contributor

@peterhashair peterhashair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great

@peterhashair peterhashair merged commit 881a085 into 4.x-dev Feb 7, 2022
@peterhashair peterhashair deleted the fixuseridtests branch February 7, 2022 20:07
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. Needs Review PRs that need a code review 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 this pull request may close these issues.

None yet

2 participants