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 test-selector-list.js tests #1

Conversation

kevinkassimo
Copy link

@kevinkassimo kevinkassimo commented Jul 31, 2018

Explicitly setting sandbox, Resource, etc.

On my local machine the errors are now gone.

(I am actually pretty amazed by the fact that these tests passes for quite a while and then suddenly explodes. Also sorry for the trouble created)

@dreamofabear dreamofabear merged commit 06e8f6d into dreamofabear:amp-list-reset-bug Jul 31, 2018
@dreamofabear
Copy link
Owner

@kevinkassimo I'd consider rewriting this test as an integration test. Manually bootstrapping services and calling element callbacks is probably brittle (and harder to read). Check out test-amp-bind.js for an example of cross-component integration tests.

@kevinkassimo
Copy link
Author

@choumx Agreed. Will attempt a replacement test later today

@kevinkassimo kevinkassimo deleted the fix-test-selector-list branch July 31, 2018 19:44
dreamofabear pushed a commit that referenced this pull request Sep 17, 2019
* Merge to local master (#1)

SmileWanted new ad code support

* Merge to local master ampproject#2

* SmileWanted new ad code support

1st version of SmileWanted ad code support

* fix lint errors
dreamofabear pushed a commit that referenced this pull request Jun 6, 2020
* [ADOCEAN-20673] remove livepreview1

* fix lint error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants