Add unit, e2e tests for amp-carousel exposed actionTrust

[caroqliu]

Continuing prior discussion, renames ampTrust as actionTrust and adds unit and e2e tests regarding this behavior.

cc @zhangsu

[estherkim]

Can you try merging master?

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[wassgha]

@googlebot I consent.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[dreamofabear]

Still looks good but thought some feedback might be helpful. 😋

[dreamofabear]

I mentioned this earlier but if we ever need to add an enum between low and default, it might be awkward (non-integer).

[caroqliu]

This is my understanding of the tradeoffs with either approach, am I missing anything?
Pros for exposing strings:

• Human-readable and meaningful, especially in debugging
• Adding intermediary values is straight forward
• Int values are an implementation detail within AMP, exposing them as such also means making others mimic this implementation.

Pros for exposing integers:

• Meaningful to compare (trust < high)
• Not localized to us -- i.e. "low" is in English
• Faster, esp. if trust is checked often

[dreamofabear]

Those look good. We could also change the integer enum values to have some breathing room -- I think they originally did before I removed it. :)

[caroqliu]

What was the reasoning behind removing it?

[dreamofabear]

Just a whim. :p

This most likely won't be necessary since 4 tiers of "trust" is quite a lot to grok. Just wanted to point out the option.

[caroqliu]

SG, going to go ahead and merge in this case.

[dreamofabear]

Ahh, sorry I just meant to extract this line rather than inline the entire helper. That way the test readers can read the logic of the test naturally like:

1. Click button
2. Wait for slideChange event
3. Assert actionTrust of event is 3

I also wonder if we could set up/tear down the listener in beforeEach/afterEach. If either of those are troublesome, just tweaking the helper a bit would be nice for semantics.

// Before
const actionTrust = await getActionTrustFor('.amp-carousel-button-next');

// After
const event = await slideChangeEventAfterClicking('.amp-carousel-button-next');
expect (event.data.actionTrust).to.equal(3); // ActionTrust.HIGH

[caroqliu]

Looks like taking and resolving the same event leads to a circular reference, so I had the helper return event.data instead. Also used the once option when adding the event listener, so should be more readable now.