Summary

Fix some test skipping issues.

Review Request #11871 — Created Nov. 10, 2021 and submitted Nov. 26, 2021, 4:10 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

There are two issues with test skipping that I ran into with my

work-in-progress M1 configuration:

Perforce tests were set up to skip if the P4 module wasn't available,

   but at some point we had an import of P4Exception snuck in that

   wasn't properly guarded. This change makes it so that we properly

   skip the test without hitting ImportError.
Once upon a time, paramiko just allowed SocketError to raise when

   port 22 was closed, but now it has its own error class.

Testing Done

Ran unit tests.

Commits

Summary	ID
Fix some test skipping issues. There are two issues with test skipping that I ran into with my work-in-progress M1 configuration: 1. Perforce tests were set up to skip if the P4 module wasn't available, but at some point we had an import of P4Exception snuck in that wasn't properly guarded. This change makes it so that we properly skip the test without hitting `ImportError`. 2. Once upon a time, paramiko just allowed `SocketError` to raise when port 22 was closed, but now it has its own error class. Testing Done: Ran unit tests.	f1a9a680513d413bffc5d747f615562a913e7865

Summary

Fix some test skipping issues.

There are two issues with test skipping that I ran into with my work-in-progress M1 configuration: 1. Perforce tests were set up to skip if the P4 module wasn't available, but at some point we had an import of P4Exception snuck in that wasn't properly guarded. This change makes it so that we properly skip the test without hitting `ImportError`. 2. Once upon a time, paramiko just allowed `SocketError` to raise when port 22 was closed, but now it has its own error class. Testing Done: Ran unit tests.

f1a9a680513d413bffc5d747f615562a913e7865

Issues

Description	From	Last Updated
F811 redefinition of unused 'P4Exception' from line 10	reviewbot	Nov. 10, 2021, 1:39 p.m.
I made a similar change for P4, except I aliased this to Exception. I don't think either approach is better …	chipx86	Nov. 25, 2021, 2:04 a.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/scmtools/tests/test_perforce.py (Diff revision 1)
The issue has been dropped. Show all issues
```
F811 redefinition of unused 'P4Exception' from line 10
```

Commits:

	Summary	ID
	Fix some test skipping issues. There are two issues with test skipping that I ran into with my work-in-progress M1 configuration: 1. Perforce tests were set up to skip if the P4 module wasn't available, but at some point we had an import of P4Exception snuck in that wasn't properly guarded. This change makes it so that we properly skip the test without hitting `ImportError`. 2. Once upon a time, paramiko just allowed `SocketError` to raise when port 22 was closed, but now it has its own error class. Testing Done: Ran unit tests.	6b0e9f27fdb43067e070a05bc51d9e2cbe662d6a
	Fix some test skipping issues. There are two issues with test skipping that I ran into with my work-in-progress M1 configuration: 1. Perforce tests were set up to skip if the P4 module wasn't available, but at some point we had an import of P4Exception snuck in that wasn't properly guarded. This change makes it so that we properly skip the test without hitting `ImportError`. 2. Once upon a time, paramiko just allowed `SocketError` to raise when port 22 was closed, but now it has its own error class. Testing Done: Ran unit tests.	f1a9a680513d413bffc5d747f615562a913e7865

Diff:

Revision 2 (+18 -24)

Show changes

	reviewboard/scmtools/tests/test_perforce.py
	reviewboard/scmtools/tests/testcases.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/scmtools/tests/test_perforce.py (Diff revision 2)

The issue has been dropped. Show all issues

I made a similar change for P4, except I aliased this to Exception. I don't think either approach is better than the other right now, but since some tests do simulate P4 operations and work without having p4python installed, I err'd on the side of keeping P4Exception operational. What do you think?

david Nov. 12, 2021, 9:44 a.m.

The only tests that aren't skipped when P4 is unavailable are PerforceAuthFormTests, which doesn't use P4Exception at all. I don't know that we gain much from being piecemeal here--if people are making changes that affect Perforce then they should have p4python and run all p4 tests.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.0.x (2ed51aa)