Create E2E tests

[martijngastkemper]

I'm trying to create a E2E test for contrib/crontab.php. I've created a work in progress PR for it: https://github.com/martijngastkemper/deployer/pull/1/files. I'm stuck in the phase of getting the tests to run properly, so don't comment on the assertions. It's definitely not finished.

testUpdatingCrontab fails with an error saying it can't find the task crontab:sync. Which is weird because testAddingCrontab can find that task:

The failed workflow run: https://github.com/martijngastkemper/deployer/pull/1/checks?check_run_id=1368346697

My first thought was an error in the test setup, but I don't get the error when I disable (comment) the testAddingCrontab function. It seems to fail just because the task is executed for the second time. I've no clue why this fails. MiscE2ETest does work with two test functions.

Can someone help me with this problem?

[antonmedv]

ping @bsacharski

[bsacharski]

Hello @martijngastkemper

The task cannot be found on the second run because the AbstractE2ETest itself has a Deployer instance, that is being initalized for every test case. When initialized, your recipe is being loaded, which in turns loads contrib/crontab recipe. However, the require_once causes the file to be loaded only once, so the task is defined only in the first Deployer instance. To be fair, it's just a side-effect of the fact that I copied this code from the unit-test base class, probably should have removed it at that time... I think you should be fine with dropping the deployer property from the AbstractE2ETest class (you may need to adjust the init() method). This would IMO make E2E tests nicer, as the Deployer is being encapsulated in the tester process, which is isolated from the main test process.

Alternatively you could hack around this by:

• adjusting test/e2e/contrib/crontab.php to use require instead of require_once for loading crontab recipe.
• adjusting contrib/crontab.php to define the parseJob function only if it doesn't exist just yet (so if (!function_exists('Deployer\\parseJob')) {).

IMO getting rid of the deployer property is a much cleaner solution, which will improve E2E tests.

Furthermore, I don't think you should use set functions in the test/e2e/CrontabE2ETest.php file - IMO this part of code should be a task in the test recipe, while the Test class itself should mostly use input/output streams to interact with the actual deploying process :)

[martijngastkemper]

Thanks for the information. I'll look into it.

[martijngastkemper]

This something of a problem. Getting rid of the deployer property doesn't matter, because Deployer\Importer\Importer::import() does a require. Loading a recipe containing a function declaration will always fail when it is imported for the second time (without a function_exists check). require isn't bound to an object instance.

Putting all functions in a helper file isn't nice, because I like the isolation of the recipes. I thought of preventing a recipe from being required multiple times by checking if it's already loaded. But I bumped in another weird problem. When calling run for the second time the output of is empty. So it's necessary to call $this->init(self::RECIPE) before every run. Apparently it's not possible to reuse the Deployer, Application or ApplicationTester instance.

Long story short. I updated the PR with the latests insights and will keep searching for a proper solution.

[martijngastkemper]

I stopped searching for a solution due to time restriction. Hopefully someone else can pick up this problem.