ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.9k stars 3.89k forks source link

amp-story: unit tests fail when "reduce-motion" setting is enabled. #35969

Open samouri opened 3 years ago

samouri commented 3 years ago

Description

npx amp unit --files=extensions/amp-story/1.0/amp-story.js fails when reduce motion is enabled. I believe this is because it is the only codepath that utilizes the Performance service which needs to be stubbed.

Error:

ERROR: '_service.Services.performanceFor(...).addEnabledExperiment is not a function [object HTMLElement]'

Source: https://github.com/ampproject/amphtml/blob/e16bbdaa94c2148ab8126238c26023ff5767675e/extensions/amp-story/1.0/amp-story.js#L456-L463

Reproduction Steps

  1. Enable "Reduced motion" in accessibility settings of your OS
  2. Run any amp story unit tests

cc @ampproject/wg-stories

gmajoulet commented 3 years ago

I think just adding addEnabledExperiment to the stub should fix it. We shouldn't be stubbing the service like this in the first place but 🤷

https://github.com/ampproject/amphtml/blob/3090428105c79898aa50f160fcd0a2c4c04db6f4/extensions/amp-story/1.0/test/test-amp-story.js#L108-L112

samouri commented 3 years ago

@gmajoulet: why shouldn't you be stubbing it? What is the ideal solution?

gmajoulet commented 3 years ago

Installing the service like this and then spy/stub on specific methods would prevent issues like this ticket.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.