Skip to content
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

add integration test for alienvault #648

Conversation

dogancanbakir
Copy link
Member

@dogancanbakir dogancanbakir commented Sep 25, 2022

This PR adds integration tests for alienvault source.

@dogancanbakir dogancanbakir changed the title add integration test for alientvault add integration test for alienvault Sep 25, 2022
Copy link
Contributor

@forgedhallpass forgedhallpass left a comment

Choose a reason for hiding this comment

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

LGTM, but the initial idea was to do the testing in a more in-depth manner, like testing the exact response to make sure it is in sync with our declared structures.

On a second thought, this might be an overkill, and it should be enough to do similar tests like your, BUT if we are going with this approach, then instead of making separate tests for all providers, we could rather write one single test that iterates through all the providers and would do the same assertions (or at least those that do not require an API key).

Would you be interested to give that a try?

For the rest we can take an approach like in: b6ed45c#diff-cef426ad95f83daa9ea90d2e3c05500731c29d2f49e7f3478b0e96aac44e46ec

@dogancanbakir
Copy link
Member Author

Testing the exact response, as you said, can be overkill because the response from the sources can be non-deterministic. As a matter of fact, integration testing is to ensure the party you're testing is still working.

Having separate test files can be seen as duplication, but it might be necessary at some point since the given API can behave differently and require different test cases (separation of concern). E.g., if you take a look at the following two cases,
1 - https://github.com/projectdiscovery/subfinder/pull/644/files#diff-33687d543eab1df5940fb81acbba5878f41d7e9e74eba1a2dfc15848ddafd022R23
2 - https://github.com/projectdiscovery/subfinder/pull/648/files#diff-2086ac43131db33420e250fa6070c53ee86475127d5168b633c0ddf31556e082R22

The first is returning subscraping.Result.Type as Subdomain at the same time as the second one returning Error.

Please let me know what you think.

@forgedhallpass
Copy link
Contributor

Testing the exact response, as you said, can be overkill because the response from the sources can be non-deterministic.

That would have been the point to discover as early as possible if the response structure has changed and our parsing can no longer handle it.

As a matter of fact, integration testing is to ensure the party you're testing is still working.

Indeed.

The first is returning subscraping.Result.Type as Subdomain at the same time as the second one returning Error.

I would say that this should not happen. My expectation is that all providers should behave the same. We have an interface for the communication, they do the magic in the implementation and return a uniform response that is later consumed by the application.

@dogancanbakir dogancanbakir deleted the alienvault_integrationtest_#594 branch June 22, 2023 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create integration test for the alienvault source
2 participants