-
Notifications
You must be signed in to change notification settings - Fork 260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit tests written for specific zendesk instance. #38
Comments
This is actually a concern we'd discussed previously as well. I'd discussed with @dpawluk and we don't believe there's a compelling reason to keep the tests tied to a specific test account. If you're interested in rewriting the tests to be generic rather than tied to a specific test instance, we'd welcome the contribution @ocke. |
Hi, We've changed the test suite to remove all of the hardcoded ID references. The test suite should now run cleanly on any Zendesk instance. We also have changed the code to support passing in Zendesk credentials via environment variables when invoking phpunit. Ideally, we would have unit tests and not just live tests, but we don't have the resources to rewrite the whole test suite at this time. |
I saw that. @shanitang did a great job 👍 |
Hey @maximeprades (and @zendesk/quokka),
I would like to chip into this project as well, but a lot of the unit tests are written in such a way it needs access to a specific (I'm guessing sandedboxy) zendesk instance.
For example this one: ForumsTest.php
It points to
$forum = $this->client->forum(22480662)->find(); // don't delete forum #22480662
While you can instead of relying on an existing ticket, move this testFind() function to below the createTest() and use the outcome of that to test the testFind().
You guys have done this for some tests. For example: TargetsTest.php.
Is there a specific reason you guys have written the test like this or can I go ahead, rewrite some of the tests, so that all test will work with any zendesk instance and doesn't rely on certain tickets or forums to exists?
If you guys give me a heads up I will add that functionality and do a PR.
The text was updated successfully, but these errors were encountered: