Add dependency checks for TFS.

Review Request #12554 — Created Aug. 19, 2022 and submitted

Information

RBTools
release-4.x

Reviewers

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.

Unit tests pass on Python 3.7-3.11.

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
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 …

daviddavid

'os' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

'rbtools.clients.errors.TooManyRevisionsError' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

'rbtools.clients.errors.InvalidRevisionSpecError' imported but unused Column: 1 Error code: F401

reviewbotreviewbot
david
  1. 
      
  2. rbtools/clients/tfs.py (Diff revision 1)
     
     
     
     
     
     
     
     
     

    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
    
  3. 
      
chipx86
Review request changed

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

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.x (291cb72)
Loading...