Skip to content

test: fix DeleteModelTest#6846

Merged
kenjis merged 8 commits intocodeigniter4:developfrom
kenjis:fix-DeleteModelTest
Nov 16, 2022
Merged

test: fix DeleteModelTest#6846
kenjis merged 8 commits intocodeigniter4:developfrom
kenjis:fix-DeleteModelTest

Conversation

@kenjis
Copy link
Copy Markdown
Member

@kenjis kenjis commented Nov 13, 2022

Description

  • fix broken test
  • add test case

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • [] User guide updated
  • Conforms to style guide

@kenjis kenjis added the testing Pull requests that changes tests only label Nov 13, 2022
Comment thread tests/system/Models/DeleteModelTest.php Outdated
* If where condition is set, beyond the value was empty (0,'', NULL, etc.),
* Exception should not be thrown because condition was explicity set
* If where condition is set, beyond the value was empty (0, '', NULL, etc.),
* Exception should not be thrown because condition was explicitly set
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What does "beyond the value was empty" mean?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure! It seems to be explicitly allowing falsy values?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added '' as a test case, because it is here in the comment.
But Postgres does not accept it.

1) CodeIgniter\Models\DeleteModelTest::testDontThrowExceptionWhenSoftDeleteConditionIsSetWithEmptyValue with data set #3 ('')
ErrorException: pg_query(): Query failed: ERROR:  invalid input syntax for type integer: ""
LINE 2: WHERE "id" = ''
                     ^

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/Postgre/Connection.php:140
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/BaseConnection.php:666
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/BaseConnection.php:593
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/BaseBuilder.php:2079
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Model.php:412
/home/runner/work/CodeIgniter4/CodeIgniter4/system/BaseModel.php:985
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Models/DeleteModelTest.php:163
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed the comment, and removed the failed test case ('').

Comment thread tests/system/Models/DeleteModelTest.php Outdated
Co-authored-by: MGatner <mgatner@icloud.com>
@kenjis kenjis merged commit 331818e into codeigniter4:develop Nov 16, 2022
@kenjis kenjis deleted the fix-DeleteModelTest branch November 16, 2022 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Pull requests that changes tests only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants