IMPORTANT: We need unit tests to merge pull requests and close issues (need help)

[barbushin]

I see many pull requests here that will probably fix many opened issues. Unfortunately I don't have a free time to test all that pull requests to be sure that they don't have a bugs and that they will not break backward compatibility.

The only one way I see is to cover 100% of code by PhpUnit tests. But, as I said I don't have a free time to write it. So I'm asking for help.

We need unit tests + Travis automation for running. It will really help to keep this package updated and extended in future.

If you have enough experience to do this - you'll be a hero. Just post your comment to this issue, mentioning that you're starting working on it, so everyone can synchronize with you.

Thank you!

[nickl-]

+1 to this project needs unit tests and if there were unit tests each patch should be required to include additional unit tests.

That said there are currently no unit tests bu there are many open issues related to bugs and fixing bugs trumps having unit tests. This issue should NOT prevent merging available fixes, if the PRs are focused enough it is not hard to see what the impact will be just by reading the changes. Besides 100% code coverage is a pipe dream in the best of cases.

The first step is realising that you do not have enough time to maintain this project on your own. The popularity has obviously grown past what a single maintainer can cope with, perhaps it is time to consider adding more maintainers or even move the project to an existing team like Respect which already has several competent PHP developers and accomplished project maintainers. This will automatically add you as a Respect team member and maintainer as well so should be worth considering.

[barbushin]

This issue should NOT prevent merging available fixes

@nickl- Sorry, but you want me to merge something without any tests? And if there will be another bugs?And what about backward compatibility?

I know this lib has many issues, but as far as many people are using it means that they can deal with currently opened issues. But I don't want to merge something and release without proper tests to make all users forced to deal with potentially broken backward compatibility and new bugs.

[barbushin]

perhaps it is time to consider adding more maintainers or even move the project to an existing team like Respect

@nickl- I don't know Respect team and I don't want to be responsible for their changes. And I don't understand why guys from Respect team must write tests for this lib? They don't use it (same as me).
You guys are using it, so if you want it to be fixed or extended in this repo, just try to write some tests. It's not a big deal actually. Otherwise, feel free to fork it and report in all issues link to your repo, so users will get a chance to try your version, on their own risk. But please don't ask me to take responsibility of somebody's risk by releasing something that couldn't be tested completely.

Reason why I can't merge currently opened pull requests is because most of them actually breaks backward compatibility, and some of them looks like will generate new bugs. And I'll be happy to merge all of them and release a new version when there will be some tests that will cover most possible risks.

This issue should NOT prevent merging available fixes

Sad thing is that 99% of open source tools users are just users, they don't really contribute anything valuable. I don't know why many users expects that repository owners MUST maintain their projects. No, we don't. I've spend x100 more time to write core of this tool then anybody who created pull request. So you think now I also MUST spend my time on testing pull requests that even doesn't looks good for me? But I even don't use this lib, actually, I don't need it. I have 10 another projects that are way more important for me.

[nickl-]

@barbushin Sorry that you feel this way, was only trying to help.

I am sure I speak for every single user when I say: Thank you very much for your efforts we all greatly appreciate what you've done so far and you are certainly under no obligation to do anything more than you are willing.

[adrium]

Automated testing would be great. I see two options:

1. Refactor Mailbox::imap() not to call the imap_* functions and to be mockable.
2. Use an actual IMAP server that contains fixtures.

Option 1 is not impossible. In my opinion, the Aura project handles faking of built-in functions quite nicely. Take a look at the Phpfunc stuff in Aura.Session as an example. This approach involves some refactoring before unit tests can be implemented. Of course, this process can introduce new bugs too and backward compatibility is not guaranteed.

Option 2 is not unit testing, but integration testing. The advantage is that the current code base can be tested as-is. Therefore, it will be possible to check if merging pull requests would introduce BC to the current code base.
The test setup is quite complex though: There needs to be an IMAP server that "hosts" the mail messages as fixtures. This approach to testing is more useful for this project in my opinion and I can contribute test cases with my profound experience in this scenario.

What approach do you prefer?

[barbushin]

As for me, approach 2. looks like what we need.

Then we need some docker container with IMAP server. Unit tests will send emails to this server by PHP mail() function or some another lib, and then check expected results by accessing it using php-imap.

I've checked, there are some docker containers with IMAP servers, and also I see that Travis supports docker containers.

[adrium]

I agree!

Unfortunately, I have never used Travis before and only have little experience with Docker.

I can help you to write some unit tests, though.

[barbushin]

On this weekend, I'll try to setup Travis with IMAP server, and few example tests. There will be a new branch tests, so you can use it as example, and make pull requests with your tests.

[Sebbo94BY]

How is it going with the tests branch or the PHPUnit tests at all? Here is a good explanation, how you can write your own PHPUnit tests: https://www.youtube.com/watch?v=k9ak_rv9X0Y

I'm currently watching the videos and hopefully, I can find some time to create some tests in order to add them here.

[Sebbo94BY]

Here you go, @barbushin: PR #292

Not much yet, but better then nothing. :)

[Sebbo94BY]

When you create new Pull Requests, please make sure, that you use the develop branch as base. :)

Travis CI is integrated and running. PHPUnit tests exists as well, but needs to be extended and improved.

[Sebbo94BY]

Only 24 open issues and 1 pull request left! Let's solve these as well. :)

[barbushin]

@Sebi94nbg God bless you man!

[Sebbo94BY]

12 issues left! :)

With #278 and #306 I need some help. This is really time consuming and difficult to fix or build a workaround.

[Sebbo94BY]

@barbushin can you please add php-imap to https://codeclimate.com/quality/? I unfortunately don't have permission to do that.

[barbushin]

@Sebi94nbg

https://codeclimate.com/github/barbushin/php-imap

[![Maintainability](https://api.codeclimate.com/v1/badges/02f72a4fd695cb7e2976/maintainability)](https://codeclimate.com/github/barbushin/php-imap/maintainability)

<a href="https://codeclimate.com/github/barbushin/php-imap/test_coverage"><img src="https://api.codeclimate.com/v1/badges/02f72a4fd695cb7e2976/test_coverage" /></a>

[Sebbo94BY]

Thanks, @barbushin! I've already added these badges to the README: ffee374

I'm unfortunately unable to change anything at https://codeclimate.com/github/barbushin/php-imap as it says, that I don't have permissions. So you would need to set up the test coverage.

I've already committed the config file with the default checks - this may needs to be updated: 5c53833

[barbushin]

Here is a test reporter ID 4eed0d135b9e1a1668a68e5e29cc71faf872937d13c42efa4faf42dad5ed3375 See https://docs.codeclimate.com/docs/configuring-test-coverage Kind Regards Sergey Barbushin

…

On Wed, May 15, 2019 at 6:19 PM TS3tools ***@***.***> wrote: Thanks, @barbushin <https://github.com/barbushin>! I've already added these badges to the README: ffee374 <ffee374> I'm unfortunately unable to change anything at https://codeclimate.com/github/barbushin/php-imap as it says, that I don't have permissions. So you would need to set up the test coverage. I've already committed the config file with the default checks - this may needs to be updated: 5c53833 <5c53833> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#286?email_source=notifications&email_token=AAFG2WAIUTVKINH3BC4KPATPVQBHFA5CNFSM4HCQ5QP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVORSNQ#issuecomment-492640566>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFG2WEUDX4DPAB43JVWWV3PVQBHFANCNFSM4HCQ5QPQ> .

[Sebbo94BY]

@barbushin I've added some configs, but it doesn't calculate the code/test coverage.

The .travis.yml seems to be fine for me.

[Sebbo94BY]

All issues are solved! :)

Now, we only have one open feature request and a ticket to build a workaround to support UTF-8 searches on stupid Microsoft Exchange servers.

Next todo is to cleanup and optimize the code to improve the maintainability of this library: https://codeclimate.com/github/barbushin/php-imap/issues

Happy coding!