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

Cover generators with tests #65

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

Conversation

KonstantinLapkovsky
Copy link

Issue #49

@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from master to 49-cover-entity-generator-with-tests December 18, 2023 08:02
@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from 49-cover-entity-generator-with-tests to master December 20, 2023 08:11
@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from master to 49-cover-entity-generator-with-tests December 20, 2023 08:11
tests/Support/Controller/ControllerMockTrait.php Outdated Show resolved Hide resolved
tests/Support/NovaTest/NovaTestMockTrait.php Outdated Show resolved Hide resolved
@DenTray
Copy link
Collaborator

DenTray commented Dec 26, 2023

@KonstantinLapkovsky please check all other mock traits according my comments

Konstantin Lapkovsky added 3 commits January 18, 2024 14:39
# Conflicts:
#	phpunit.xml
#	tests/Support/NovaTestMockTrait.php
@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from 49-cover-entity-generator-with-tests to master January 18, 2024 13:02
@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from master to 49-cover-entity-generator-with-tests January 18, 2024 13:02
return $this->mockNativeFunction('\\RonasIT\\Support\\Generators', 'class_exists', $result);
}

public function mockCheckingNonExistentNovaPackageExistence(): Mock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function mockCheckingNonExistentNovaPackageExistence(): Mock
public function mockCheckingNovaPackageExistence(bool $result = false): Mock

@@ -0,0 +1,75 @@
<?php

namespace RonasIT\Support\Tests\Support\Controller;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace RonasIT\Support\Tests\Support\Controller;
namespace RonasIT\Support\Tests\Support\ControllerGeneratorTest;

@@ -0,0 +1,66 @@
<?php

namespace RonasIT\Support\Tests\Support\Shared;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace RonasIT\Support\Tests\Support\Shared;
namespace RonasIT\Support\Tests\Support;

@@ -0,0 +1,74 @@
<?php

namespace RonasIT\Support\Tests\Support\NovaResource;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace RonasIT\Support\Tests\Support\NovaResource;
namespace RonasIT\Support\Tests\Support\NovaResourceGeneratorTest;

@@ -0,0 +1,126 @@
<?php

namespace RonasIT\Support\Tests\Support\NovaTest;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace RonasIT\Support\Tests\Support\NovaTest;
namespace RonasIT\Support\Tests\Support\NovaTestGeneratorTest;

return $this->mockClassExistsFunction(false);
}

public function classExistsMethodCall(?string $path, ?string $className, bool $result = true): array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like optional args are using only in the mockResourceGeneratorForNonExistingNovaResource method. Could we use filled args there and remove optional here?

@DenTray DenTray removed their assignment Jan 22, 2024
@KonstantinLapkovsky KonstantinLapkovsky changed the base branch from 49-cover-entity-generator-with-tests to master January 25, 2024 09:30
$this->removeRecursivelyGeneratedFolders(getcwd() . '/tests/vfs:');
}

protected function removeRecursivelyGeneratedFolders(string $path): void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move it to the parent class, but before please try to mock mk_dir_recursive function, as we discussed, it may be helpfull and able you to remove this logic at all

Comment on lines 87 to 88
$this->removeRecursivelyGeneratedFolders(getcwd() . '/vfs:');
$this->removeRecursivelyGeneratedFolders(getcwd() . '/tests/vfs:');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we implement clearVFS and remove all available paths inside?

$this->assertEquals($this->getDatabaseAssertionData(), $fields);
}

public function getFieldsMock(): array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move it to the fixture

];
}

public function getDatabaseAssertionData(): array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use such array inside the test cases without additional method

Comment on lines 14 to 27
public function mockControllerGeneratorForExistingController(): void
{
$this->mockClass(ControllerGenerator::class, [
$this->classExistsMethodCall('controllers', 'PostController')
]);
}

public function mockControllerGeneratorForNotExistingService(): void
{
$this->mockClass(ControllerGenerator::class, [
$this->classExistsMethodCall('controllers', 'PostController', false),
$this->classExistsMethodCall('services', 'PostService', false)
]);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as discussed, let's remove this methods and call such logic inside the tests

]);
}

public function mockConfigurations(): void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems we no need in this, let's try to define getPackageProviders method inside the base test case


trait GeneratorMockTrait
{
public function mockNativeFunction(string $namespace, string $name, $result): Mock
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check that such logic already exists in the laravel helpers package and use it

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.

None yet

2 participants