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

[Issue] Let background commands actually run in the background #38359

Closed
5 tasks done
m2-assistant bot opened this issue Jan 18, 2024 · 4 comments · Fixed by #37511
Closed
5 tasks done

[Issue] Let background commands actually run in the background #38359

m2-assistant bot opened this issue Jan 18, 2024 · 4 comments · Fixed by #37511
Labels
Area: Framework Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch

Comments

@m2-assistant
Copy link

m2-assistant bot commented Jan 18, 2024

This issue is automatically created based on existing pull request: #37511: Let background commands actually run in the background


Update 2023-05-30

After discussion with @engcom-Charlie (thanks!), I set up the Magento Functional Testing Framework locally to investigate the specifics of this test. The report was that the test worked fine on the 2.4-develop branch, but was failing on #32690, so it was suggested that the changes there were breaking this test, and that the test was fine as-is on 2.4-develop. I thought this was a race condition, but it seems it's more complicated than a simple race condition.

The reason the test isn't failing on 2.4-develop is because STDERR isn't being redirected for the child processes, so the testing framework wasn't actually generating background processes when it should have been. 7d56fe4 fixes that bug, which will mean that all other unreliable tests will now be failing here. Many of the test failures here will be fixed by magento/magento2-functional-testing-framework#904

Description

While working on #32690 (comment), it was discovered that the test StorefrontVerifyProductAfterPartialReindexOnSeveralWebsitesTest (from Magento\AcceptanceTest\_default\Backend\StorefrontVerifyProductAfterPartialReindexOnSeveralWebsitesTest) is unreliable. This pull request aims to improve the reliability of this test. While this is not a complete fix, it will improve the reliability.

Why is the test unreliable?
The test aims to ensure that indexers are correctly triggered / rebuilt when making changes in the admin. The general process goes: log into admin, make a change, run cron, witness result. It's the "run cron" step that is causing issues. The internals of that step spawns separate processes, and then returns. These separate processes will run in the background and will work, however there is a race condition between these background processes running to completion and the next step of the test being carried out. When the indexer process is quick, all is well. When the test framework gets to the next step before the background process complete, the test fails.

How does this change help?

This pull request turns the background process into a foreground process, so the test framework won't ever reach the next step until the "background" process finishes.

Why is this change incomplete?

There is no guarantee that a job has actually been scheduled. There is still an expectation that there is a indexer_update_all_views job in the queue, ready to be executed. Usually this will be the case, however it's not guaranteed. Further test improvements are welcome in this area.

Manual testing scenarios

Run the following PHP script. It should report success.

#!/usr/bin/env php
<?php

use Magento\Framework\App\ObjectManager;
use Magento\Framework\Console\Cli;
use Magento\Framework\Shell\CommandRendererBackground;

$command = 'sleep 10';

require __DIR__ . '/app/bootstrap.php';
new Cli('Magento CLI');
$objectManager = ObjectManager::getInstance();
$commandRenderer = $objectManager->get(CommandRendererBackground::class);

$command = $commandRenderer->render($command);
$startTime = microtime(true);

echo "DEBUG: running `$command` ...\n";
$process = proc_open($command, [
    0 => ['pipe', 'r'],
    1 => ['pipe', 'w'],
    2 => ['pipe', 'w'],
], $pipes, getcwd());
fclose($pipes[0]); // STDIN
echo stream_get_contents($pipes[1]); // STDOUT
echo stream_get_contents($pipes[2]); // STDERR
proc_close($process);

$endTime = microtime(true);
$duration = $endTime - $startTime;
$result = $duration >= 5 ? 'fail' : 'pass';
printf('Test %sed. It took %.1f seconds to start a long-running background process.', $result, $duration);
echo "\n";

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)
@m2-assistant m2-assistant bot added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Jan 18, 2024
@m2-community-project m2-community-project bot added this to Pull Request In Progress in High Priority Backlog Jan 18, 2024
@engcom-Charlie engcom-Charlie added Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Jan 18, 2024
@github-jira-sync-bot
Copy link

Unfortunately, not enough information was provided to create a Jira ticket. Please make sure you added the following label(s): Reproduced on 2.4.x, ^Area:.*

Once all required labels are present, please add Issue: Confirmed label again.

@github-jira-sync-bot github-jira-sync-bot removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Jan 18, 2024
@engcom-Charlie engcom-Charlie added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Jan 18, 2024
@github-jira-sync-bot
Copy link

Unfortunately, not enough information was provided to create a Jira ticket. Please make sure you added the following label(s): Reproduced on 2.4.x, ^Area:.*

Once all required labels are present, please add Issue: Confirmed label again.

@github-jira-sync-bot github-jira-sync-bot removed the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Jan 18, 2024
@engcom-Charlie engcom-Charlie added Area: Framework Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Jan 18, 2024
@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.adobe.com/browse/AC-10863 is successfully created for this GitHub issue.

Copy link
Author

m2-assistant bot commented Jan 18, 2024

✅ Confirmed by @engcom-Charlie. Thank you for verifying the issue.
Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Framework Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: done Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch
Projects
Development

Successfully merging a pull request may close this issue.

2 participants