fix test cases #47
fix test cases #47
Conversation
|
@mostafa-hisham Thanks so much for all your work on these unit tests too. It would be great if we could circle back on this to get it all wrapped up. I just added it to the current sprint. |
There was a problem hiding this comment.
Yay! Thank so much for fixing this @mostafa-hisham !
I just have a question about the autoloader classes for prod vs dev.
| @@ -105,6 +105,7 @@ | |||
| 'AWPCP_Container' => $baseDir . '/includes/class-container.php', | |||
| 'AWPCP_ContainerConfiguration' => $baseDir . '/includes/class-container-configuration.php', | |||
| 'AWPCP_ContainerConfigurationInterface' => $baseDir . '/includes/interface-container-configuration.php', | |||
| 'AWPCP_ContainerConfigurationTestCase' => $baseDir . '/tests/includes/class-container-configuration-test-case.php', | |||
There was a problem hiding this comment.
Do we need to add something to the release compiler to remove the test autoloader classes from production? Or do you think it's fine to leave here, even if the classes don't exist? It looks like the dev dependencies are listed here too.
There was a problem hiding this comment.
We can add --no-dev flag when running composer install or dumpautoload
it will not include dev dependencies
There was a problem hiding this comment.
Sounds like a great plan! Would you mind adding this in so we don't have the dev dependencies included in the production code?
There was a problem hiding this comment.
I ran the composer with --no-dev and pushed the changes
but I really think that we should not push changes in vendor to the repo and we need to make a release or a production code
we should run composer install --no-dev
There was a problem hiding this comment.
That's a great plan. Maybe we should start with setting it up this way in BD first? I think it's fine to leave this one as is for now.
tests/suite/admin/listings/test-complete-listing-table-view.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @mostafa-hisham ! The code looks great on this. Do you think we need any extra testing for the composer file changes?
|
@stephywells I don't think so. |
|
Sounds good! Feel free to merge this whenever you're ready. |
|
@stephywells I can not merge this pull request
|
|
Oops! Sorry about that @mostafa-hisham. This is fixed now. |
@stephywells In this PR I tried to fix Unit tests as much as I could.
now only 18 tests are failing with me locally.
I am not sure if this PR will fix the patchwork error https://github.com/Strategy11/another-wordpress-classifieds-plugin/actions/runs/4484949580/jobs/7885969046
but we can test it and see if it will help or not