Summary

Add dependency checks for TFS.

Review Request #12554 — Created Aug. 19, 2022 and submitted Sept. 5, 2022, 2:01 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-4.x

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This updates TFSClient to add formal support for dependency checks.
Unlike most other SCM updates, this one's a bit more complicated due to
the three wrapper classes that can be chosen.

The wrapper classes now all inherit from a base class, which defines the
interface for wrappers. They then override their own
check_dependencies(), performing the checks that used to be done in
TFSClient.

TFSClient has been updated to check each wrapper in order, selecting
one that passes dependency checks, or failing with a hopefully helpful
error listing broad dependencies.

That has the added benefit of now allowing a fallback to TEE when the TF
Helper is installed but Java is not.

The tf_wrapper attribute is now a property, which enforces a
dependency check, emitting a warning if dependencies haven't already
been checked via setup() or has_dependencies(). All methods on
TFSClient go through this already, ensuring this is used.

There weren't any TFS unit tests before. This change adds the first
ones, which are all focused on the variety of dependency checks.

Testing Done

Unit tests pass on Python 3.7-3.11.

Commits

Summary	ID
Add dependency checks for TFS. This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.	a1f884cab7f306b5c6029f9d283eaf2600fd5c06

Summary

Add dependency checks for TFS.

This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.

a1f884cab7f306b5c6029f9d283eaf2600fd5c06

Issues

Description	From	Last Updated
I think it would be less confusing to do: for ...: wrapper = wrapper_cls(**wrapper_kwargs) try: wrapper.check_dependencies() found = True break …	david	Aug. 23, 2022, 6:26 p.m.
'os' imported but unused Column: 1 Error code: F401	reviewbot	Aug. 23, 2022, 6:29 p.m.
'rbtools.clients.errors.TooManyRevisionsError' imported but unused Column: 1 Error code: F401	reviewbot	Aug. 23, 2022, 6:29 p.m.
'rbtools.clients.errors.InvalidRevisionSpecError' imported but unused Column: 1 Error code: F401	reviewbot	Aug. 23, 2022, 6:29 p.m.

flake8 passed.

JSHint passed.

rbtools/clients/tfs.py (Diff revision 1)

The issue has been resolved. Show all issues

I think it would be less confusing to do:

for ...:
    wrapper = wrapper_cls(**wrapper_kwargs)

    try:
        wrapper.check_dependencies()

        found = True
        break
    except SCMClientDependencyError:
        # Skip this one. Go to the next.
        continue

Change Summary:


Fixed the wrapper dependency check loop to be structured a lot nicer.
Fixed bad assumptions on the return string type from executing tf.exe, and improved testing around this.

Commits:

	Summary	ID
	Add dependency checks for TFS. This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.	4924f37823cf2e9127674df12e8ebe7f876d1759
	Add dependency checks for TFS. This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.	fa64eb92062ae7e4c1df387d94e9c4617849d0d3

Diff:

Revision 2 (+1468 -142)

Show changes

	rbtools/clients/tfs.py
	rbtools/clients/tests/test_tfs.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/clients/tests/test_tfs.py (Diff revision 2)
The issue has been resolved. Show all issues
```
'os' imported but unused

Column: 1
Error code: F401
```

rbtools/clients/tests/test_tfs.py (Diff revision 2)

The issue has been resolved. Show all issues

'rbtools.clients.errors.TooManyRevisionsError' imported but unused

Column: 1
Error code: F401

rbtools/clients/tests/test_tfs.py (Diff revision 2)

The issue has been resolved. Show all issues

'rbtools.clients.errors.InvalidRevisionSpecError' imported but unused

Column: 1
Error code: F401

Change Summary:

Removed unused imports accidentally scheduled for commit.

Commits:

	Summary	ID
	Add dependency checks for TFS. This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.	fa64eb92062ae7e4c1df387d94e9c4617849d0d3
	Add dependency checks for TFS. This updates `TFSClient` to add formal support for dependency checks. Unlike most other SCM updates, this one's a bit more complicated due to the three wrapper classes that can be chosen. The wrapper classes now all inherit from a base class, which defines the interface for wrappers. They then override their own `check_dependencies()`, performing the checks that used to be done in `TFSClient`. `TFSClient` has been updated to check each wrapper in order, selecting one that passes dependency checks, or failing with a hopefully helpful error listing broad dependencies. That has the added benefit of now allowing a fallback to TEE when the TF Helper is installed but Java is not. The `tf_wrapper` attribute is now a property, which enforces a dependency check, emitting a warning if dependencies haven't already been checked via `setup()` or `has_dependencies()`. All methods on `TFSClient` go through this already, ensuring this is used. There weren't any TFS unit tests before. This change adds the first ones, which are all focused on the variety of dependency checks.	a1f884cab7f306b5c6029f9d283eaf2600fd5c06

Diff:

Revision 3 (+1462 -142)

Show changes

	rbtools/clients/tfs.py
	rbtools/clients/tests/test_tfs.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.x (291cb72)