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

Added tests for api requests #78

Merged
merged 3 commits into from
May 30, 2024
Merged

Added tests for api requests #78

merged 3 commits into from
May 30, 2024

Conversation

Alopalao
Copy link
Contributor

@Alopalao Alopalao commented May 29, 2024

Closes #38
Closes #46

Summary

Added tests for API request from main.py
Assigned version 1.0.0 to requests

Local Tests

Made requests to test versioning

End-to-End Tests

Results from e2e PR

+ python3 -m pytest tests/test_e2e_40_sdntrace.py --reruns 2 -r fEr
============================= test session starts ==============================
platform linux -- Python 3.11.2, pytest-8.1.1, pluggy-1.5.0
rootdir: /tests
plugins: rerunfailures-13.0, timeout-2.2.0, anyio-4.3.0
collected 14 items

tests/test_e2e_40_sdntrace.py ..............                             [100%]

=============================== warnings summary ===============================

- Assigned version 1.0.0 to requests
@Alopalao Alopalao requested a review from a team as a code owner May 29, 2024 16:49
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

Excellent to have unit test cov for these endpoints, @Alopalao.

1 - If you can also refactor e2e test_e2e_40_sdntrace.py, the e2e test cases were using the unversioned endpoints:

tests/test_e2e_40_sdntrace.py
149:                api_url = KYTOS_API + '/amlight/sdntrace/trace'
189:        api_url = KYTOS_API + '/amlight/sdntrace/trace'
236:        api_url = KYTOS_API + '/amlight/sdntrace/trace'
335:        api_url = KYTOS_API + '/amlight/sdntrace/trace'
382:        api_url = KYTOS_API + '/amlight/sdntrace/trace'
822:        api_url = KYTOS_API + '/amlight/sdntrace/trace'
866:        api_url = KYTOS_API + '/amlight/sdntrace/trace'

2 - The exec on scrutinizer there was an unexpected traceback, check it out if it's deterministic or not:

https://scrutinizer-ci.com/g/kytos-ng/sdntrace/inspections/a8ba7476-7b75-4849-8035-ca7101cb799e/log

tests/unit/tracing/test_trace_manager.py::TestTraceManager::test_is_entry_invalid_not_colored
  /home/scrutinizer/build/.tox/py311/lib/python3.11/site-packages/_pytest/unraisableexception.py:80: PytestUnraisableExceptionWarning: Exception ignored in thread started by: <bound method TraceManager._spawn_trace of <napps.amlight.sdntrace.tracing.trace_manager.TraceManager object at 0x7f9bee815990>>
  
  Traceback (most recent call last):
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/trace_manager.py", line 94, in _spawn_trace
      tracer = TracePath(self, trace_id, trace_entries)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/tracer.py", line 47, in __init__
      self.init_switch = self.get_init_switch()
                         ^^^^^^^^^^^^^^^^^^^^^^
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/tracer.py", line 56, in get_init_switch
      return Switches().get_switch(self.init_entries.dpid)
                                   ^^^^^^^^^^^^^^^^^^^^^^
  AttributeError: 'str' object has no attribute 'dpid'
  
    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

setup.py Show resolved Hide resolved
@viniarck
Copy link
Member

viniarck commented May 29, 2024

Also, UI client request endpoints need to be updated too (here's one line for example):

Copy link
Contributor

@italovalcy italovalcy left a comment

Choose a reason for hiding this comment

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

LGTM!

@Alopalao
Copy link
Contributor Author

This exception in the tests:

tests/unit/tracing/test_trace_manager.py::TestTraceManager::test_is_entry_invalid_not_colored
  /home/scrutinizer/build/.tox/py311/lib/python3.11/site-packages/_pytest/unraisableexception.py:80: PytestUnraisableExceptionWarning: Exception ignored in thread started by: <bound method TraceManager._spawn_trace of <napps.amlight.sdntrace.tracing.trace_manager.TraceManager object at 0x7f9bee815990>>
  
  Traceback (most recent call last):
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/trace_manager.py", line 94, in _spawn_trace
      tracer = TracePath(self, trace_id, trace_entries)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/tracer.py", line 47, in __init__
      self.init_switch = self.get_init_switch()
                         ^^^^^^^^^^^^^^^^^^^^^^
    File "/home/scrutinizer/build/.tox/py311/var/lib/kytos/napps/amlight/sdntrace/tracing/tracer.py", line 56, in get_init_switch
      return Switches().get_switch(self.init_entries.dpid)
                                   ^^^^^^^^^^^^^^^^^^^^^^
  AttributeError: 'str' object has no attribute 'dpid'

It is caused because TestManager class creates a thread which is constantly running. It does not fail in the specified test test_is_entry_invalid_not_colored. Looks like it fails around the time that test is running. Changing the order of tests will change the shown test in this warning.

@viniarck viniarck self-requested a review May 30, 2024 17:05
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

Nicely done @Alopalao.

Regarding the unit test thread issue, that's a sign that that part isn't easy and graceful to test, so in the future it deserves a refactoring, but it's a not blocker, so map in issue for it, but continue on the other ones that it's on your radar, OK? Thanks

Feel free to merge this and the e2e test PR.

@Alopalao Alopalao merged commit 477b9e3 into master May 30, 2024
1 check passed
@Alopalao Alopalao deleted the testing/api branch May 30, 2024 17:28
@Alopalao
Copy link
Contributor Author

Leaving this comment as reference that here was fulfilled kytos issue #487

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.

sdntrace isn't testing any of the @rest routes with get_test_client (eventually) add v1 on API routes
3 participants