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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes unit and feature test suite #12640

Merged
merged 31 commits into from
Mar 9, 2023
Merged

Conversation

marcusmoore
Copy link
Collaborator

This PR gets the test suite back to a runnable and green state!

To test it for yourself follow the updated docs in TESTING.md (basically cp .env.testing. .env.testing and update values).

I made an effort to get the configuration closer to what Laravel ships by default. This includes:

  • Updating phpunit.xml
  • Removing the existing .env.testing file, adding it to .gitignore, and instructing users to copy the new .env.testing.example file to .env.testing and populate it with their testing database information
  • Consolidating the /tests/Unit/BaseTest.php and /tests/TestCase.php into Laravel's standard /tests/TestCase.php. TestCase.php has the following characteristics:
    • LazilyRefreshDatabase is defined at this level (more details below)
    • The SecurityHeaders middleware is disabled for all tests. This is because it calls header_remove() which causes problems for some requests in tests
  • Standardizing method casing by converting snake cased method names to camel

LazilyRefreshDatabase details: this trait that is provided by Laravel has been added to the TestCase that all tests extend. It works similar to RefreshDatabase but only triggers a database refresh right before a database query is executed. This means pure unit tests that don't interact with the database do not incur a performance hit. But the time to start running the first test is still "slow" because the trait runs migrate:fresh before the first test but then starts using a transaction so subsequent tests are faster. I'd like to explore ways to improve this at at later date.

Something to note: The tests are run from a pure state meaning each test (or possibly group of tests using setup()) need to build the world for their use case. In the future we can explore seeding the database with data that is standard for all tests.

Another note: This PR doesn't do any work to migrate the Codeception tests 馃槵 (that can happen next)

@what-the-diff
Copy link

what-the-diff bot commented Mar 8, 2023

  • Removed the RefreshDatabase trait from TestCase.php
  • Added LazilyRefreshDatabase to TestCase.php
  • Updated UsersForSelectListTest to use lazily refresh database instead of refreshing it on every test run (this was causing a lot of issues with tests failing randomly)
  • Change the class name from BaseTest to TestCase
  • Remove all protected $tester; and public function _before() in each test file
  • Add use Tests\TestCase; at top of each test file
  • Replace 'use Illuminate\Foundation\Testing\DatabaseMigrations' with 'use DatabaseTransactions'; (in some files)
  • Changed the class name from BaseTest to TestCase
  • Removed all references of $tester and UnitTester
  • Added test prefixes for each method in SnipeModelTest, StatuslabelTest, UserTest classes
  • Replaced DatabaseMigrations with RefreshDatabase trait in StatusLabel and User tests

@snipe
Copy link
Owner

snipe commented Mar 9, 2023

This is awesome - thank you so much, @marcusmoore!!

@snipe snipe merged commit 65c2f27 into snipe:develop Mar 9, 2023
@marcusmoore marcusmoore deleted the fix/test-suite branch March 9, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants