Summary

Add typing for the functions in TestCase.

Review Request #14035 — Created July 13, 2024 and submitted Nov. 5, 2024, 5:03 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This updates TestCase to add type hints all throughout. This helps
ensure we pass in the right typse to functions, and that we get the
right transport and client types when setting up state for API tests.

Testing Done

Unit tests pass.

Commits

Summary	ID
Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	98ad375cdd0626813277d9f77586bde3b28e185a

Issues

Description	From	Last Updated
'typing.Any' imported but unused Column: 1 Error code: F401	reviewbot	July 13, 2024, 2:13 a.m.
redefinition of unused 'RBClient' from line 17 Column: 5 Error code: F811	reviewbot	July 13, 2024, 2:13 a.m.
While you're here, mind adding docstrings and changing to super()?	david	Oct. 10, 2024, 3:53 p.m.
Here too.	david	Oct. 10, 2024, 3:53 p.m.
Now that we're properly using __future__.annotations, we should change from Dict to dict and List to list.	david	Nov. 1, 2024, 2:32 a.m.
Blank line between these two.	david	Nov. 1, 2024, 2:32 a.m.
Can we add a doc comment for this?	david	Nov. 1, 2024, 2:33 a.m.
maxDiff doesn't appear to be used anywhere within the rbtools codebase. Can we just get rid of this?	david	Nov. 1, 2024, 2:33 a.m.
Should probably be Any instead of object.	david	Nov. 1, 2024, 2:35 a.m.

flake8 failed.

JSHint passed.

flake8

rbtools/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
'typing.Any' imported but unused

Column: 1
Error code: F401
```
rbtools/testing/testcase.py (Diff revision 1)
The issue has been resolved. Show all issues
```
redefinition of unused 'RBClient' from line 17

Column: 5
Error code: F811
```

Change Summary:

Removed an unused import and a duplicate import.

Commits:

	Summary	ID
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	6006d70d24e42d186300de3f86073b6b931b2b3b
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	9c81b7bfff3bec145667b91c415b0f60a6734587

Diff:

Revision 2 (+126 -46)

Show changes

rbtools/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/testing/testcase.py (Diff revision 2)

The issue has been resolved. Show all issues

While you're here, mind adding docstrings and changing to super()?

rbtools/testing/testcase.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Here too.
```

Change Summary:


Added some docstrings to the setup/teardown methods.
Added additional type hints for class/instance variables.
Simplified super() calls.

Commits:

	Summary	ID
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	9c81b7bfff3bec145667b91c415b0f60a6734587
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	9ae2883847e5d422970e673033ec2840462c31be

Diff:

Revision 3 (+216 -62)

Show changes

rbtools/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/testing/testcase.py (Diff revision 3)

The issue has been resolved. Show all issues

Now that we're properly using __future__.annotations, we should change from Dict to dict and List to list.

rbtools/testing/testcase.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Blank line between these two.
```
rbtools/testing/testcase.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can we add a doc comment for this?
```

rbtools/testing/testcase.py (Diff revision 3)

The issue has been dropped. Show all issues

maxDiff doesn't appear to be used anywhere within the rbtools codebase. Can we just get rid of this?

chipx86 Nov. 1, 2024, 2:35 a.m.

This tells TestCase that we don't want a limit when comparing strings, dictionaries, lists, etc. By default it's a low number. We can either set to a number or use None to let it just compare without truncation messages. None ends up being a good option the majority of the time in our cases.

rbtools/testing/testcase.py (Diff revision 3)

The issue has been dropped. Show all issues

Should probably be Any instead of object.

chipx86 Nov. 1, 2024, 2:35 a.m.

object is considered safer than Any if you want type checking, since Any just turns off type checking for the variable. object fits with the usage of these methods and still gives us type checking so that, say, config['foo'] + 1 would generate a type error instead of silently appearing to work. Sometimes Any is appropriate, but probably not here.

Change Summary:


Switched from Dict and List to dict and list, respectively.
Added a blank line between typing imports.
Added a comment for ws_re.

Commits:

	Summary	ID
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	9ae2883847e5d422970e673033ec2840462c31be
	Add typing for the functions in TestCase. This updates `TestCase` to add type hints all throughout. This helps ensure we pass in the right typse to functions, and that we get the right transport and client types when setting up state for API tests.	98ad375cdd0626813277d9f77586bde3b28e185a

Diff:

Revision 4 (+226 -70)

Show changes

rbtools/testing/testcase.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.x (cfb4adc)