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

Fix Regression Failures; Type Error Handling Improvements; Strict Type Checking Updates #246

Merged
merged 4 commits into from
Oct 13, 2020

Conversation

sdimitro
Copy link
Contributor

Commit 1

mypy: disallow untyped calls for the sdb directory

There was recent work in drgn where all the helpers were annotated
with types. With that work in place we no longer need to allow
untyped calls.

Commit 2 (Fixes Regression - original PR from Matt here: #210)

Walker should check for canonicalized type names

As we know, drgn type equality does not work right, so we need to
compare canonicalized type names. When combined with openzfs/zfs#10236,
zfs_dbgmsg now works on ztest core dumps.

Serapheim Note:
With drgn changing its type rules once again this commit is now needed
for existing tests to not fail.

Commit 3 (Fixes Regression)

update regression output

drgn recently updated its error messages when it comes to page
table lookups to match the latest terminology used by the kernel.

Commit 4 (Fixes Regression; Error-handling improvements with new tests)

ptype: remove anonymous union test and improve error handling

In the past we used to assume lazy type rules from drgn and printing
an anonymous union or struct from its typedef just worked.
Unfortunately, with the latest updates for types in drgn, this no
longer works so part of this commit removes the expectation of
this functionality from our tests. We may want to introduce this
functionality again in the future implemented in a different way.
Fortunately, these kind of types are rare.

The second part of this commit improves the error handling of
looking up types.

sdimitro and others added 4 commits September 25, 2020 15:08
There was recent work in drgn where all the helpers were annotated
with types. With that work in place we no longer need to allow
untyped calls.
As we know, drgn type equality does not work right, so we need to
compare canonicalized type names. When combined with openzfs/zfs#10236,
`zfs_dbgmsg` now works on ztest core dumps.

Serapheim Note:
With drgn changing its type rules once again this commit is now needed
for existing tests to not fail.
drgn recently updated its error messages when it comes to page
table lookups to match the latest terminology used by the kernel.
In the past we used to assume lazy type rules from drgn and printing
an anonymous union or struct from its typedef just worked.
Unfortunately, with the latest updates for types in drgn, this no
longer works so part of this commit removes the expectation of
this functionality from our tests. We may want to introduce this
functionality again in the future implemented in a different way.
Fortunately, these kind of types are rare.

The second part of this commit improves the error handling of
looking up types.
@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2020

Codecov Report

Merging #246 into master will decrease coverage by 0.00%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #246      +/-   ##
==========================================
- Coverage   87.38%   87.37%   -0.01%     
==========================================
  Files          60       60              
  Lines        2473     2488      +15     
==========================================
+ Hits         2161     2174      +13     
- Misses        312      314       +2     
Impacted Files Coverage Δ
sdb/commands/ptype.py 100.00% <ø> (ø)
sdb/commands/internal/util.py 90.24% <88.88%> (-2.07%) ⬇️
sdb/command.py 73.57% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76813d8...f0bbc58. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants