Test / fix e2e overlapping reports

[ChristiaanScheermeijer]

Description

Because we use a function to wrap the JWP and Cleeng scenarios, the CodeceptJS run workers' reporting fails. The reporter instance exists only once for each worker and it creates two Allure runs within the same instance because of the duplicate Feature(`subscription - ${providerName}`) calls.

I've played with data-driven tests, but this will run all steps sequentially.

The easiest fix was to either remove the Feature and Before calls from the runTestSuite functions so it only creates 1 test run.

After this change, the Allure reporting will show all the scenarios before this was the last one completed.

https://github.com/codeceptjs/allure-legacy/blob/8c31bd2d0736487edaa7d92a813562b2571cef49/index.js#L180-L182

Calls:

Allure.prototype.startSuite = function(suiteName, timestamp) {
    this.suites.unshift(new Suite(suiteName, timestamp));
};

Allure.prototype.endSuite = function(timestamp) {
    var suite = this.getCurrentSuite();

    suite.end(timestamp);
    if(suite.hasTests()) {
        writer.writeSuite(this.options.targetDir, suite);
    }
    this.suites.shift();
};

The culprit... This function only returns the first suite:

Allure.prototype.getCurrentSuite = function() {
    return this.suites[0];
};

[github-actions]

Visit the preview URL for this PR (updated for commit d8f0b21):

https://ottwebapp--pr495-fix-e2e-overlapping-on02e5ih.web.app

_{(expires Sat, 11 May 2024 11:58:55 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: c198f8a3a199ba8747819f7f1e45cf602b777529}

[ChristiaanScheermeijer]

I was playing with a different idea.

It is possible to customise the parallel execution of tests in CodeceptJS. Creating a folder for all integration tests and running these with different configurations would be a nice and scalable solution.

• platforms/web/test-e2e/tests/integrations
• platforms/web/test-e2e/tests/integrations/login_test.ts
• platforms/web/test-e2e/tests/integrations/register_test.ts
• platforms/web/test-e2e/tests/integrations/payment_test.ts
• platforms/web/test-e2e/tests/integrations/subscription_test.ts

These suites will have to be split for two integration options:

const integrations = ['jpw', 'cleeng'];

// Create two configs and inject the integration (name for now)
const configs = integrations.map((integrationName) => ({
  inject: {
    integrationName,
  },
}));

// for each integration, spawn a worker and register the test group containing all integration tests 
// can also be split over multiple workers...
for (const config of configs) {
  const worker = workers.spawn();
  worker.addTests(testGroups[0]);
  worker.addConfig(config);
}

The rest of the files are spread over multiple workers like normal and don't need any special configuration.

[dbudzins]

Does it not work to just move the Before to the top level where you moved Feature? This seems like an otherwise nice, simple enough fix, but I wish we didn't need to copy all the await beforeScenario calls. Seems too easy to forget to add when you create a new Scenario.

[dbudzins]

Oh I see, Before depends on the runTestSuite args...

[ChristiaanScheermeijer]

I don't like the beforeScenario much either, we could also make this a custom step: I.amLoggedIn(config);

We also might be able to refactor the accountContext to a shared value. We can then remove that from each test as well.

[dbudzins]

I was playing with a different idea.

@ChristiaanScheermeijer This looks promising and more maintainable to me. Any downsides?

[ChristiaanScheermeijer]

@ChristiaanScheermeijer This looks promising and more maintainable to me. Any downsides?

I was playing with this yesterday and I got it (almost) working. The run workers script does feel a bit too custom because I have to trick codecept into duplicating the integration tests with a different config.

The biggest downside is that the e2e tests will only test 1 integration when you run them via the UI or with steps (easier to debug). This is because the run-workers script changes the codecept config before spawning them.

This is what I got so far:

I created two integration configurations because injection works via require and no inline objects :')

The codecept config can either have a predefined include, or we need to ignore the tests by default (when running steps or UI):

    },
  },
  include: {
    I: './utils/steps_file.ts',
    integration: `./configs/jwp`,
  },
  bootstrap: null,

The run-workers script creates two config groups with overrides:

// Create two configs and inject the integration (name for now)
const configs = integrations.map((integrationName) => ({
  include: {
    integration: `./configs/${integrationName}`,
  },
}));

Currently, 4 workers are spawned, but this needs to be worked out a bit more:

for (const config of configs) {
  for (const group of testGroups) {
    const worker = workers.spawn();
    worker.addTests(group);
    worker.addConfig(config);
    worker.addOptions({ config: __dirname + '/codecept.desktop.ts', verbose: true });
  }
}

The challenge here is to balance everything out. If we only create two workers for the integrations, the rest of the tests are distributed over the remainder of the workers.

However, we could also run the non-integration tests on the integration-configured worker...

[ChristiaanScheermeijer]

@dbudzins do you think this idea still has potential? We are looking into optimizing the e2e tests (duration), but this might change based on this outcome.

[dbudzins]

@dbudzins do you think this idea still has potential? We are looking into optimizing the e2e tests (duration), but this might change based on this outcome.

Hm, it's getting a bit fiddly now.... it looks like just splitting the files fixes the reporting issue. It's kinda annoying, but seems like a quick easy fix: #503

For whatever reason it also looks like they still won't run in parallel, but since there are a lot of tests to run, I think this will shake out in the wash.

What do you think?

[ChristiaanScheermeijer]

@dbudzins As of now, I'm doubting between keeping it "as is" or proceeding with #503. I can pick up on #503, submit a PR and close this one?

[AntonLantukh]

@ChristiaanScheermeijer both approaches ("as is" AND "separate files in 503") are fine for us. You can choose the one which is easier to maintain. Having custom workers logic looks too complex for our case.

[AntonLantukh]

@ChristiaanScheermeijer any preference here?