-
Notifications
You must be signed in to change notification settings - Fork 46
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 test requirements un(der)documented #78
Comments
Hi @hn3000 - Thanks for your interest in contributing. While mocking and isolating code under test is a standard best practice, we don't believe it is useful for this scenario. The tests in this library are here to ensure the this library properly integrates with the Postmark API, so it's an exception to the general practice. We use TravisCI to do these integration tests using servers that are pre-populated with data. If you'd like to run the tests locally, these are the necessary env vars, which can also be stored as keys in a file called If a readme on how to configure these would be useful, that's something we can add, but we probably won't add any mocks to the tests, as again, this significantly reduces the usefulness of the tests, and increases the overhead in adding new endpoints and tests to this library. We understand that getting a full test suite to pass locally may be more effort than it's worth, but if you have a specific need with the library, please feel free to open an issue/PR. We will happily review and potentially accept these patches after we do our own verification/addition of tests, but large-scale refactors are probably unnecessary. Hopefully this is helpful, but please feel free to share your thoughts or contact us in support if you'd like to more guidance on contributing. |
A Readme explaining how to configure the environment vars would be very useful. With regards to mocking, I have had good results with mocked http APIs, so in your case there we would implement very basic mocks of the routes used in the unit tests. That would not make the integration test against the actual service redundant but would allow some coverage for unit testing. If the mocks do some minimal checking (like validation of input against an openapi spec), these tests will catch stupid mistakes and typos without being to annoying. A good alternative to these mocks would be a testing account on your systems that could be configured to not actually send emails, but I can understand if you're concerned about such an account enabling DOS attacks on your infrastructure. |
As a potential contributor I cloned this repository and tried to run all unit tests to ensure my setup is working correctly.
This results in massive failure:
The README does not mention any specific instructions for running the tests, the wiki it refers to does not mention anything either.
A closer look at the failures reveals that the failures assert that configuration settings are provided which are not included in the repository. A search for the configuration variables does not show any examples or documentation about them in this repository.
An example for a similar configuration can be found in the postmark-php repository, but it does not elucidate on the expectations for the tokens that need to be configured, either.
I can guess, what an
WRITE_ACCOUNT_TOKEN
is, but what exactly is the required configuration for theREAD_SELENIUM_OPEN_TRACKING_TOKEN
?I am also not quite comfortable to give unit tests a valid account token with write permissions, would a mock not make more sense for unit tests? (Or did I guess wrong what that token above should be?)
Could you please provide some more information about the unit tests and how to run them?
Would you be open to a PR for a version of the unit tests that do not require the actual postmark back-end to run?
The text was updated successfully, but these errors were encountered: