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

Test is not finished when expect().toPass() exceed test timeout #111

Closed
danielsobon opened this issue Jun 28, 2023 · 8 comments · Fixed by #118
Closed

Test is not finished when expect().toPass() exceed test timeout #111

danielsobon opened this issue Jun 28, 2023 · 8 comments · Fixed by #118
Assignees
Labels
bug Something isn't working
Milestone

Comments

@danielsobon
Copy link

danielsobon commented Jun 28, 2023

Test setup:

├── @playwright/[email protected]
├── @reportportal/[email protected]

How to reproduce

  1. Set up test timeout to 20 sec
const config: PlaywrightTestConfig = {
  globalSetup: require.resolve('./global-setup'),
  /* Maximum time one test can run for. */
  timeout: 20 * 1000,
}
  1. Create a test with any of repeatable check for 30 sec
  • expect.poll()
  • expect().toPass()
import { expect, test } from '@playwright/test';

test.describe('Expect', () => {
  test(`Expect pool @desktop`, async () => {
    await expect
      .poll(
        () => {
          return 1;
        },
        { timeout: 30_000 }
      )
      .toBe(2);
  });
  test('Expect toPass @desktop', async () => {
    await expect(() => {
      expect(1).toBe(2);
    }).toPass({ timeout: 30_000 });
  });
});
  1. Configure RP to includeTestSteps: true

Actual result

Test & test step (expect) are still in progress, so the entire run is also not finished
image

@AmsterGet AmsterGet added the bug Something isn't working label Jul 11, 2023
@danielsobon
Copy link
Author

hey @AmsterGet, any update regarding the issue? I'm asking because we realized the problem is more serious than we thought.
Because of fact the assertion cannot be finished we don't receive report, nor valid status of entire playwright run.

  • None of standard playwright report (json/html/github) is generated when we have integrated reportPortal
  • Github job does not catch the failures (says he run is green even if we have failed tests)

image

@AmsterGet
Copy link
Member

Hey @danielsobon!
Thanks for pointing this out!
Our team has begun to investigate this issue.
We will keep you updated on this thread.

@Bam6ycha Bam6ycha self-assigned this Jul 26, 2023
@Bam6ycha
Copy link
Contributor

Bam6ycha commented Aug 4, 2023

Hey @danielsobon.
This issue has been successfully fixed and will be available in the next version of the agent.
But I have several questions.
Why are you using timeout inside the test greater than your global timeout?
Does your provided example only reproduce the bug, or is it the real case?

If it is only to reproduce, and you have such tests which run as long as the global timeout exceeds we recommend splitting it into small pieces and running them in parallel.

@danielsobon
Copy link
Author

@Bam6ycha yep :) We have tests that consist of many steps. Each of them can take some time. In case of bad performance of application, all combined steps can reach the test timeout, so this is a real case.

Second thing, what you can also reproduce is, when we reach expect timeout during preforming operation, example API request.
Example code (I recommend to set 2-3 retries because it might be reproduced in random run):

import test, { expect, request } from '@playwright/test';

test.describe('Expect @desktop @smoke', () => {
  test.only('Expect toPass @desktop @smoke', async () => {
    await expect(async () => {
      const status = await sendRequestStep();
      expect(status).toBe(400); //status returns 200
    }).toPass({ timeout: 5_000 });
  });
});

async function sendRequestStep() {
  return await test.step(`I get available pets status response`, async () => {
    const context = await request.newContext({
      baseURL: 'https://petstore.swagger.io',
    });
    const status = await context
      .get('/v2/pet/findByStatus?status=available', {})
      .then((response) => response.status())
      .finally(async () => await context.dispose());
    return status;
  });
}

RP view
image

@AmsterGet AmsterGet added this to the 5.2.2 milestone Aug 6, 2023
@AmsterGet AmsterGet modified the milestones: 5.2.2, 5.2.0 Aug 6, 2023
@danielsobon
Copy link
Author

hey @AmsterGet do you know when I could expect new release?

@AmsterGet
Copy link
Member

Hi @danielsobon !
I suppose in a few days.
Stay tuned, we'll notify you here.

@AmsterGet AmsterGet modified the milestones: 5.2.0, 5.1.3 Aug 29, 2023
@AmsterGet AmsterGet linked a pull request Sep 7, 2023 that will close this issue
@AmsterGet
Copy link
Member

Hi @danielsobon !
Please check the version 5.1.3.

@danielsobon
Copy link
Author

@AmsterGet , @Bam6ycha I've already retested it. Problem does not occur anymore, thx for fix! 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants