Using sys.platform makes testing harder

[miketheman]

Since the agent performs a sys.platform lookup to determine the OS it is running on, this is not exercising alternate platforms when running tests on any one platform.

[josePhoenix]

In particular https://github.com/DataDog/dd-agent/blob/master/tests/test_system.py#L111 is not a place we need to check which platform the tests are running on

[miketheman]

Also ref #537 - testing an added platform is nigh impossible.

[alq666]

@miketheman I'm not sure I understand the issue here. Platform-dependent code needs to run based on detection at runtime. What is the alternative to using sys.platform?

[miketheman]

@alq666 Why the close? Not understanding the problem doesn't solve the problem, only closes the ticket. 🌘

To elaborate further on the problem, eunning the test suite on your Mac will exercise the OSX-specific behaviors, not Linux, running on Travis will execute Debianoids, and neither will test Windows.

To run all tests efficiently on any platform, for any other platform, we should have an abstraction where during the test, one can specify what platform the test should mock against, and execute those, instead of requiring a full test on every platform ever for testing units of parsing logic.

There should still be a process of testing on obscure platforms and edge cases.

[miketheman]

Might be worthwhile to look at tox testing, where this is baked in:
https://testrun.org/tox/latest/config-v2.html?highlight=platform#example-generating-and-selecting-variants

[alq666]

To be clear, this affects the testing code, not the core code, doesn't it? The title and the description imply that we should modify the agent code so that some obscure tests pass.

My 2 gripes with this issue:

1. The requirement to be able to test any target on any source platform strikes me as needlessly expensive. I don't understand what it gets us except complication and false confidence based on mock code that may or may not reflect the actual target platform. To me there is no sense of unit-testing platform-specific code on anything else but the target platform.
2. Corollary to 1, spinning instances of any target platform is cheap.

[alq666]

@miketheman I hope this clarifies my position.

[miketheman]

@alq666 thanks for clarifying.
Sorry if the description was unclear, the code itself at runtime makes direct calls to sys.platform, making stubbing/mocking that for testing impossible.

My concern is that while developing, tests are run against the system that a user is on, not the branching/parsing logic that exists for per-platform behavior. The cycle until that code is actually tested on all platforms is long, and can introduce bugs, that could be caught earlier during the development cycle.

If there's a concern with mock behavior, then why do we have it at all? It provides a useful piece of tooling, to provide known inputs and expected outputs - and we've added more test data as we get it.

Also, just found that in #615 a class was added to allow for Platform helpers, so we should probably move towards using those instead of peppering out code with calls to sys.platform.

If there's no use in unit testing platform-specific code, then we will always need to add, maintain, and cycle through target platforms - a facility that we don't have today.