pradosoft / prado

Prado - Component Framework for PHP
Other
187 stars 70 forks source link

Spot TDbCronModuleTest test failure #918

Closed ctrlaltca closed 1 year ago

ctrlaltca commented 1 year ago

Sometimes a TDbCronModuleTest unit test fails.

1) TDbCronModuleTest::testLogCronTask
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'testTaskAA'
+'testTaskDD'

/home/runner/work/prado/prado/tests/unit/Util/Cron/TDbCronModuleTest.php:246
/home/runner/work/prado/prado/vendor/bin/phpunit:123

You can see an example here: https://github.com/pradosoft/prado/actions/runs/4822368307/jobs/8589506193

belisoful commented 1 year ago

I've noticed this. I have a fix for this in the up coming Cron v2 update... I've been wildly distracted from that portion... but it's still waiting in the wings.

I think this issue come from an SQL order that isn't happening and two elements are rarely "randomly" gets switched. The unit test could be "corrected" to look for elements in not just static places... but the new Cron code should fix this.

belisoful commented 1 year ago

833 I do believe this fixes the issue... But it isn't complete yet. Getting the behaviors updated was very high priority, while cron was much lower. I was hoping to have this updated quickly after the last cron time fix but went down a huge rabbit hole.

belisoful commented 1 year ago

Let me clarify. The presumption in DB Cron is that when a DB record is inserted, it will be returned in order of insertion and so the SQL is not ordered by Record UID in getting the records because of this presumption.

As we are seeing, the DB is not always returning the same order as insertion. I don't know why. However, the new Cron V2 does order by insertion id in this case.

belisoful commented 1 year ago

If you are looking for a quick fix, one of the SQL SELECT statements "just" needs to be ordered by the UID. I'm sure it would be easy to nail down given the unit test that is failing and seeing the cron method producing the wrong results.

Besides properly ordering by UID, Cron v2 lazy loads the jobs from the DB rather than loads them all. Only those needed are loaded. It can handle a much larger load of cron jobs. That is to say, it makes the cron system ready for a user base rather than "just for system use" or low usage.

belisoful commented 1 year ago

I went ahead and zinged this spot error. I do believe this fixed the issue.

belisoful commented 1 year ago

ok. didn't fix the issue. This time:

1) TDbCronModuleTest::testLogCronTask
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'testTaskAA'
+'testTaskBB'

/2022prado/protected/vendor/pradosoft/prado/tests/unit/Util/Cron/TDbCronModuleTest.php:246
/2022prado/protected/vendor/pradosoft/prado/vendor/bin/phpunit:123
belisoful commented 1 year ago

ok. NOW I'm sure it's fixed.