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

feat: adding dynamic scenario and object creation tests for CARLA #227

Merged
merged 43 commits into from
Jul 12, 2024

Conversation

abanuelo
Copy link
Contributor

@abanuelo abanuelo commented Apr 1, 2024

Description

Adding the following three test cases:

  1. Car applying throttle increases speed
  2. Car applying break will bring the car to a halt
  3. Can create all Carla objects. For this third one, there was an error with the BusStop class within model.scenic that needed to be captured. Also note that these tests are automatically skipped as there are memory leak errors with Carla at the moment!

Issue Link

Checklist

  • I have tested the changes locally via pytest and/or other means
  • I have added or updated relevant documentation
  • I have autoformatted the code with black and isort
  • I have added test cases (if applicable)

Additional Notes

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 20.73171% with 65 lines in your changes missing coverage. Please review.

Project coverage is 86.15%. Comparing base (1924fbc) to head (c21239b).
Report is 18 commits behind head on main.

Files Patch % Lines
tests/simulators/carla/test_actions.py 18.03% 50 Missing ⚠️
tests/simulators/carla/test_blueprints.py 25.00% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #227      +/-   ##
==========================================
- Coverage   86.16%   86.15%   -0.02%     
==========================================
  Files         144      146       +2     
  Lines       25007    25285     +278     
==========================================
+ Hits        21548    21784     +236     
- Misses       3459     3501      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abanuelo abanuelo changed the title feat: [wip] adding dynamic scenario tests for CARLA feat: adding dynamic scenario and object creation tests for CARLA Apr 3, 2024
@abanuelo abanuelo marked this pull request as ready for review April 3, 2024 15:37
@abanuelo abanuelo requested review from dfremont and Eric-Vin April 3, 2024 15:39
@abanuelo
Copy link
Contributor Author

Important!!!!

  1. In order to trigger the dynamic scenario tests for CARLA we need to have an environment variable exported called CARLA_ROOT. As part of the simulator CI, this environment variables will be exported as necessary.
  2. The pyproject.toml file has been updated such that the only precondition to install the carla pypi package is if we have python version below 3.10. Python 3.11 is giving Carla some problems: Carla fails to install in Python 3.11 venv carla-simulator/carla#7328

Copy link
Collaborator

@dfremont dfremont left a comment

Choose a reason for hiding this comment

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

Looks good overall -- a few suggestions above.

@abanuelo abanuelo requested review from Eric-Vin and dfremont July 11, 2024 18:38
Copy link
Collaborator

@dfremont dfremont left a comment

Choose a reason for hiding this comment

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

A few more comments (and one which I've unresolved from before).

@abanuelo abanuelo requested a review from dfremont July 11, 2024 22:40
Copy link
Collaborator

@dfremont dfremont left a comment

Choose a reason for hiding this comment

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

Looking good: just a couple last tweaks.

@abanuelo abanuelo requested a review from dfremont July 11, 2024 23:58
Copy link
Collaborator

@dfremont dfremont left a comment

Choose a reason for hiding this comment

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

Looks great!

@dfremont dfremont merged commit 6c3bc2b into main Jul 12, 2024
24 of 26 checks passed
@dfremont dfremont deleted the abanuelo/add-carla-dynamic-scenario-tests branch July 12, 2024 17:13
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.

3 participants