bavix / laravel-wallet

It's easy to work with a virtual wallet
https://bavix.github.io/laravel-wallet/
MIT License
1.15k stars 230 forks source link

Failing tests #588

Closed martin-ro closed 2 years ago

martin-ro commented 2 years ago

Describe your task When running my tests I get Working inside an embedded transaction is not possible.

To Reproduce Steps to reproduce the behavior: Create a test:

<?php

use App\Models\User;
use function Pest\Laravel\actingAs;
use function Pest\Laravel\get;

uses()->group('WalletTest');

test('fails', function () {
    $user = User::factory()->asStudent()->create();

    actingAs($user)
        ->get('/')
        ->assertOk();
});

test('works', function () {
    $user = User::factory()->asStudent()->create();

    get('/')->assertOk();
});

Trace Error

 Working inside an embedded transaction is not possible. https://bavix.github.io/laravel-wallet/#/transaction

  at tests/Feature/WallteTest.php:14
     10▕     $user = User::factory()->asStudent()->create();
     11▕ 
     12▕     actingAs($user)
     13▕         ->get('/')
  ➜  14▕         ->assertOk();
     15▕ });
     16▕ 
     17▕ test('works', function () {
     18▕     $user = User::factory()->asStudent()->create();

  Tests:  1 failed, 1 passed
  Time:   4.52s

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Server:

Additional context I've read https://github.com/bavix/laravel-wallet/issues/463 but this behavior still doesn't really make any sense to me. What DB transaction is not closed using actingAs?

rez1dent3 commented 2 years ago

Hello, @martin-ro. Inside the package for performance, a kind of saga is implemented. The work is described here #412 and here #564. Tests (pest) start with a transaction, which prevents correct verification. The package itself controls the race condition, locks and rolls back the state in case of an error in the transaction. Unfortunately, there are no other solutions available at the moment. This is a technical limitation that had to be taken to improve fault tolerance and performance.

martin-ro commented 2 years ago

Hey @rez1dent3, thanks for taking the time to explain. I figured out that in my case tests are failing because I display a wallet balance in the front end.

Currently, that's the only use case where I have trouble. Is there a way to get to the wallet balance without triggering an atomic lock? I think it would be helpful if the package offered some convenient method to do some operations without locks. Something like $user->wallet->withoutBlock()->balance or $user->wallet->copy()->balance.

Do you think there is some way to get this to work?

It's really a significant problem, the wallet obviously needs atomic locks but also I can't use it if I can't run the test suite anymore 😅

rez1dent3 commented 2 years ago

@martin-ro If you are interested in the balance within the tests, then get directly from the table. This can be done in the following way:

$wallet->getOriginalBalanceAttribute();

You can try to write your own implementation of LockServiceInterface and change it through the config, but most likely you will have to change the DatabaseServiceInterface as well.


I have ideas how I can drop my implementation of the DatabaseServiceInterface, but it is entirely framework dependent. I need a "committing" event for the commit method inside ManagesTransactions. Without this change, I will have to leave the current solution. Here is a link to the PR in the framework: https://github.com/laravel/framework/pull/44608

The class can also be redefined inside the package, but there is a high probability of not keeping track of the changes. And this will negatively affect the support of the package.

martin-ro commented 2 years ago

In my case I got all tests working with $wallet->getOriginalBalanceAttribute();. Thanks!

rez1dent3 commented 2 years ago

Hello, @martin-ro. Tag 9.6.0. In theory, the problem should go away

martin-ro commented 2 years ago

Thanks a lot!