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

Add integration tests for <amp-analytics type=googleanalytics> #19674

Merged
merged 7 commits into from Dec 7, 2018

Conversation

lannka
Copy link
Contributor

@lannka lannka commented Dec 5, 2018

Closes #19708

Copy link
Contributor

@torch2424 torch2424 left a comment

Choose a reason for hiding this comment

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

LGTM 🚢 😄

"account": "UA-67833617-1"
},
"requests": {
"host": "${RequestBank.getUrl(1)}"
Copy link
Member

Choose a reason for hiding this comment

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

I thought we were going to name these?

document.cookie = '_ga=;expires=' + new Date(0).toUTCString();
});

it('should assign new cid', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have test to get stored cid? Or is that not possible to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the test above is parsing CID from cookie

Copy link
Contributor

Choose a reason for hiding this comment

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

Missed that. Thanks.

@lannka
Copy link
Contributor Author

lannka commented Dec 7, 2018

Decided to move the CLIENT_ID related tests out of this googleanalytics specific tests #19709

@lannka lannka merged commit 857fe14 into ampproject:master Dec 7, 2018
@lannka lannka deleted the googleanalytics-integration-test branch December 7, 2018 22:53
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
…oject#19674)

* Add integration tests for <amp-analytics type=googleanalytics>

* remove

* skip one failing amp-bind test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants