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 sync and async test guide at contributing.md #2684

Closed
zhihali opened this issue Jul 9, 2024 · 2 comments · Fixed by #2694
Closed

add sync and async test guide at contributing.md #2684

zhihali opened this issue Jul 9, 2024 · 2 comments · Fixed by #2694

Comments

@zhihali
Copy link
Contributor

zhihali commented Jul 9, 2024

What problem do you want to solve?

at contrib repo, usually the current test is sync tests, but we sometimes need to run async test. If we just add a async test in our current test class, the test will automatically skip the async test. This will lead to async test pass in anytime.

an example could be this one:
Under redis instrumentation, I want to add an async test, to test if I run set under a watch process, what will happen. If I just add it under the class we already have, it will pass in any case.

class TestRedis(TestBase):
    def setUp(self):
        super().setUp()
        RedisInstrumentor().instrument(tracer_provider=self.tracer_provider)

    def tearDown(self):
        super().tearDown()
        RedisInstrumentor().uninstrument()

    async def redis_operations(self):
        r = redis.Redis()

        async with r.pipeline(transaction=False) as pipe:
            await pipe.watch("a")

            # Simulate a change by another client
            await r.set("a", "bad")

            pipe.multi()
            await pipe.set("a", "1")

            await pipe.execute()

    @pytest.mark.asyncio
    async def test_redis_operations(self):
        # Run the redis_operations function
        await self.redis_operations()

        # Retrieve the spans
        spans = self.memory_exporter.get_finished_spans()

        # Check that the last span has the correct properties
        assert spans[-1].status.status_code == trace.StatusCode.UNSET

        # Check for specific attributes in the span
        assert any(event.name == "WatchError" for event in spans[-1].events)

Describe the solution you'd like

The solution of test issue is create an async test for it, and the new async test should iInherited from IsolatedAsyncioTestCase
we should add some guide at contributing guide, which could save much time for new rookie contributors(like myself) to debug

Describe alternatives you've considered

not sure if we should add some annotations at test file about this thing

Additional Context

No response

Would you like to implement a fix?

Yes

@emdneto
Copy link
Member

emdneto commented Jul 10, 2024

@zhihali I think we can have a simple test instruction in https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/CONTRIBUTING.md#guideline-for-instrumentations . A PR to state this in CONTRIBUTING.md would be nice.

@zhihali
Copy link
Contributor Author

zhihali commented Jul 10, 2024

@zhihali I think we can have a simple test instruction in https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/CONTRIBUTING.md#guideline-for-instrumentations . A PR to state this in CONTRIBUTING.md would be nice.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants