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

Integration tests #180

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Integration tests #180

wants to merge 15 commits into from

Conversation

dgirardi
Copy link
Collaborator

@dgirardi dgirardi commented Sep 30, 2022

Integration tests; this aims to be exhaustive, but is a WIP.

Cases covered so far:

In progress:

  • New style (ortb) native adUnits and templates
  • Banner on app/mobile

Missing:

Not supported by PUC (!)

  • Native on AMP or app

@dgirardi dgirardi force-pushed the playwright-test branch 18 times, most recently from 0c446a5 to dbe7a19 Compare September 30, 2022 16:08
@dgirardi dgirardi changed the title Initial integration tests Integration tests Oct 11, 2022
@dgirardi dgirardi marked this pull request as ready for review October 27, 2022 18:18
@musikele
Copy link
Contributor

The biggest issue with this PR is that running tests is aleatory. Basically, all tests may fail because of network, cpu, bandwith, google deciding that we're overcalling their endpoint, and other thousand million causes that it'll be impossible to say "oh, tests are failing because it's my fault".

Another issue is that these tests take several minutes to run. On a local pc they may take up to 5-6 minutes. So they're only suitable for a CI environment.

So, probably if we remove some tests, or some browsers, and we keep the run timing under 2 minutes, and we tweak the parameters so that tests failing go to zero, this is the test suite we were missing

@patmmccann patmmccann requested review from ChrisHuie and removed request for musikele September 7, 2023 18:18
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.

3 participants