scenario_runner icon indicating copy to clipboard operation
scenario_runner copied to clipboard

Fix unit test and code check actions

Open TayYim opened this issue 8 months ago • 0 comments

0. Description

This PR aims to fix the github actions of scenario_runner, which includes:

  • Unit test
  • Static Code Analysis
    • Check Code Format
    • Check Code Quality

In the previous several PRs, these actions failed.

This PR fixes Unit Test and Check Code Format.

There are tons of code changes, mainly contributed by autopep8.

If you want to check code changes only related to Unit Test, see commit

  • https://github.com/carla-simulator/scenario_runner/commit/58656a37ced8dd4499d220afb5312606bff1aa88
  • https://github.com/carla-simulator/scenario_runner/commit/1bd6d09f9a4b1fe76091c847f45c590f4b1c425f
  • https://github.com/carla-simulator/scenario_runner/commit/6aca6bf01271e4449709f66366b1cab9177c5199

1. Unit Test

I will show what issues I see and what changes I made here.

1.1 ConstantVelocityAgent

Commit: https://github.com/carla-simulator/scenario_runner/commit/6aca6bf01271e4449709f66366b1cab9177c5199

Unit Test failed because it can't import ConstantVelocityAgent.

Just copy the same file from Carla 0.9.15 to srunner/tests/carla_mocks/agents/navigation/constant_velocity_agent.py.

1.2 WeatherParameters

Commit: https://github.com/carla-simulator/scenario_runner/commit/1bd6d09f9a4b1fe76091c847f45c590f4b1c425f

In srunner/scenarioconfigs/scenario_configuration.py L107:

self.weather = carla.WeatherParameters(sun_altitude_angle=70, cloudiness=50)

Here, the initialization parameters were passed to WeatherParameters. However, in srunner/tests/carla_mocks/carla.py, WeatherParameters did not have an init method. Therefore, I added an init method.

1.3 Infinite waiting for map

In scenario_runner/srunner/tests/test_xosc_load.py:

config = OpenScenarioConfiguration(filename, client, {})

Will wait forever. It turns out that there's a dead-loop in scenario_runner/srunner/scenarioconfigs/openscenario_configuration.py L209:

self.client.load_world(self.town)
while self.client.get_world().get_map().name.split('/')[-1] != self.town:
    continue

self.client.get_world().get_map().name gets nothing. So this loop never end.

The cause of this problem is that the World and Map classes do not define read and write operations for map_name in srunner/tests/carla_mocks/carla.py. Therefore, I have added the relevant setters and getters.

1.4 ActorConfigurationData init

Error msg:

File "scenario_runner/srunner/tools/openscenario_parser.py", line 681, in convert_position_to_transform
raise AttributeError("Object '{}' provided as position reference is not known".format(obj))
AttributeError: Object 'hero' provided as position reference is not known

It happens because in srunner/scenarioconfigs/openscenario_configuration.py, the ActorConfigurationData is initialized like

ActorConfigurationData(
            model=model, transform=None, rolename=rolename, speed=speed, color=color, category=category, args=args)

transform is None. I think it should be a default carla.Transform().

After I change None to carla.Transform(), issue has gone.

[!WARNING] I'm not sure whether this modification is correct.

1.5 Empty actor list

srunner/tools/openscenario_parser.py L1281:

actor_transform = OpenScenarioParser.convert_position_to_transform(position)

It doesn't put an actor_list into the params. If it's a relative position, there will be no actor to be referenced.

Fixed:

actor_transform = OpenScenarioParser.convert_position_to_transform(position, config.other_actors + config.ego_vehicles)

2. Check Code Format

At local, I use this command to format python code in the whole project:

autopep8  --in-place --recursive . --max-line-length=120 --ignore=E731

I have also tried with the format commands in .github/workflows/static_code_check.yml:

autopep8 srunner/scenariomanager/*.py --in-place --max-line-length=120 --ignore=E731
autopep8 srunner/scenariomanager/scenarioatomics/*.py --in-place --max-line-length=120
autopep8 srunner/scenarios/*.py --in-place --max-line-length=120
autopep8 srunner/tools/*.py --in-place --max-line-length=120
autopep8 srunner/scenarioconfigs/*.py --in-place --max-line-length=120
autopep8 scenario_runner.py --in-place --max-line-length=120
autopep8 srunner/autoagents/*.py --in-place --max-line-length=120

However, even my code is well-formatted for sure, the Check Code Format actions fails.

Originally, the Check Code Format action check whether the code is met up to the pep8 standard by:

git diff --quiet HEAD --; if [ ! $? -eq 0 ]; then echo "Code is not autopep8 compliant. Please run code_check_and_formatting.sh"; git diff HEAD --; fi

This command works well locally, but always failed to execute in GitHub action.

Therefore, I decided to use a third-party action for autopep8 format checking directly: https://github.com/peter-evans/autopep8

The code in .github/workflows/static_code_check.yml is much simpler:

    - name: Check Code Format
      uses: peter-evans/[email protected]
      with:
        # Arguments to pass to autopep8
        args: --max-line-length=120 --ignore=E731

It works well then.

3. Check Code Quality

Well, this is hard.

Pylint has reported numerous points for optimization, including missing-function/module/class-docstring, unused-variable, attribute-defined-outside-init, unnecessary-lambda, among others.

I am unsure whether I should address each issue individually or adjust the Pylint configuration to suppress certain warnings.


This change is Reviewable

TayYim avatar Jun 11 '24 03:06 TayYim