Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pytest suite using httpd+nghttpx, a start #10175

Closed
wants to merge 1 commit into from
Closed

Conversation

icing
Copy link
Contributor

@icing icing commented Dec 28, 2022

Basic infra to run curl against an apache httpd plus nghttpx for h3.

- adding '--with-test-httpd=<path>' to configure non-standard apache2 install
- python env and base classes for running httpd
- basic tests for connectivity with h1/h2/h3

tests/httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/httpd/testenv/certs.py Outdated Show resolved Hide resolved
tests/httpd/testenv/certs.py Outdated Show resolved Hide resolved
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added files in docs/examples/* by mistake!

tests/httpd/config.ini.in Outdated Show resolved Hide resolved
tests/httpd/conftest.py Outdated Show resolved Hide resolved
tests/httpd/test_01_basic.py Outdated Show resolved Hide resolved
tests/httpd/testenv/__init__.py Outdated Show resolved Hide resolved
tests/httpd/testenv/__init__.py Outdated Show resolved Hide resolved
@icing
Copy link
Contributor Author

icing commented Dec 28, 2022

You added files in docs/examples/* by mistake!

More examples are good! (removed them again, sorry)

tests/httpd/testenv/certs.py Outdated Show resolved Hide resolved
@icing icing requested a review from bagder Dec 29, 2022
tests/httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/httpd/testenv/curl.py Outdated Show resolved Hide resolved
]
if alpn_proto is not None:
if alpn_proto not in self.ALPN_ARG:
raise f'unknown ALPN protocol: "{alpn_proto}"'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid Exception: Expression `"unknown ALPN protocol


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

tests/tests-httpd/testenv/curl.py Outdated Show resolved Hide resolved
valid_from: timedelta = timedelta(days=-1),
valid_to: timedelta = timedelta(days=89),
client: bool = False,
sub_specs: Optional[List['CertificateSpec']] = None):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 9 similar findings have been found in this PR


Invalid type: Expression typing.Optional[typing.List[tests.tests.__sub__(httpd.testenv.certs.CertificateSpec)]] is not a valid type.


🔎 Expand here to view all instances of this finding
File Path Line Number
tests/tests-httpd/testenv/certs.py 118
tests/tests-httpd/testenv/certs.py 164
tests/tests-httpd/testenv/certs.py 167
tests/tests-httpd/testenv/certs.py 188
tests/tests-httpd/testenv/certs.py 192
tests/tests-httpd/testenv/certs.py 196
tests/tests-httpd/testenv/certs.py 196
tests/tests-httpd/testenv/certs.py 200
tests/tests-httpd/testenv/certs.py 200

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

with open(fpath) as fd:
return load_pem_private_key("".join(fd.readlines()).encode(), password=None)

def load_credentials(self, name: str, key_type=None, single_file: bool = False, issuer: Credentials = None):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

35% of developers fix this issue

💬 12 similar findings have been found in this PR


Incompatible variable type: issuer is declared to have type Credentials but is used as type None.


🔎 Expand here to view all instances of this finding
File Path Line Number
tests/tests-httpd/testenv/certs.py 356
tests/tests-httpd/testenv/certs.py 356
tests/tests-httpd/testenv/certs.py 356
tests/tests-httpd/testenv/certs.py 372
tests/tests-httpd/testenv/certs.py 373
tests/tests-httpd/testenv/certs.py 376
tests/tests-httpd/testenv/certs.py 446
tests/tests-httpd/testenv/certs.py 471
tests/tests-httpd/testenv/curl.py 46
tests/tests-httpd/testenv/curl.py 47

Showing 10 of 12 findings. Visit the Lift Web Console to see all.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@classmethod
def create_root(cls, name: str, store_dir: str, key_type: str = "rsa2048") -> Credentials:
store = CertStore(fpath=store_dir)
creds = store.load_credentials(name="ca", key_type=key_type, issuer=None)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

61% of developers fix this issue

💬 6 similar findings have been found in this PR


Incompatible parameter type: Expected Credentials for 3rd parameter issuer to call CertStore.load_credentials but got None.


🔎 Expand here to view all instances of this finding
File Path Line Number
tests/tests-httpd/testenv/certs.py 340
tests/tests-httpd/testenv/certs.py 344
tests/tests-httpd/testenv/certs.py 388
tests/tests-httpd/testenv/certs.py 482
tests/tests-httpd/testenv/certs.py 522
tests/tests-httpd/testenv/curl.py 240

Visit the Lift Web Console to find more details in your report.


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

tests/tests-httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/tests-httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/tests-httpd/testenv/curl.py Outdated Show resolved Hide resolved
tests/tests-httpd/testenv/curl.py Outdated Show resolved Hide resolved
lib/http2.c Outdated Show resolved Hide resolved
args.extend(["--cacert", self.env.ca.cert_file])

if force_resolve and u.hostname and u.hostname != 'localhost' \
and not re.match(r'^(\d+|\[|:).*', u.hostname):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

55% of developers fix this issue

Incompatible parameter type: Expected `Variable[typing.AnyStr <


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

tests/tests-httpd/testenv/nghttpx.py Outdated Show resolved Hide resolved
@icing
Copy link
Contributor Author

icing commented Jan 9, 2023

If CI works through this, I think it is a good first version to merge.

- adding '--with-test-httpd=<path>' to configure non-standard apache2 install
- python env and base classes for running httpd
- basic tests for connectivity with h1/h2/h3
- adding test cases for truncated responses in http versions.
- adding goaway test for HTTP/3.
- adding "stuttering" tests with parallel downloads in chunks with varying delays between chunks.
- adding a curltest module to the httpd server, adding GOAWAY test.
    - mod_curltest now installs 2 handlers
      - 'echo': writing as response body what came as request body
      - 'tweak': with query parameters to tweak response behaviour
- marked known fails as skip for now
@bagder
Copy link
Member

bagder commented Jan 9, 2023

Thanks!

@bagder bagder closed this in 33ac97e Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants