Skip to content

Commit

Permalink
Merge #2857 #2858
Browse files Browse the repository at this point in the history
2857: Fix leaks of ≈21 more wallets in integration tests r=rvl a=Anviking

- [x] Replace 3 manual calls to `Link.postWallets` with the `ResourceT` `postByronWallet`
- [x] Also make `createWalletFromPublicKeyViaCLI` run in `ResourceT`

### Comments

- 21 wallets is a pretty significant number, so will be good to fix. Although the leaks were at the end of the tests, which lessens the impact.
- There seem to be _some_ concurrency related leaks.

#### Before second fix

- --match "WALLET" -j 1 ->18 leaks, all at the end

#### After fixes

- --match "WALLET" -j 1, no leaks
- --match "WALLET" -j 8, 10 leaks (3 BUSY, 6 others)

```
SQLITE_BUSY in logs:
ica.14828b804a1ebe804caddd1abb5d314d2be0a62a.sqlite
ica.25341683360a040721f587d069b95762b302f608.sqlite
rnd.5fc0960622bc4d3a5d366a0920dfd9b5ef578ced.sqlite

rnd.81273c47add77e98b3993e01cab8b2a836cd48b7.sqlite
she.95cff5e68f07b759c72d4da4f1aa96e0b101bba2.sqlite
she.a99218ce4bbc77f05617d642c0595246621b67b6.sqlite

Remaining DBs:
ica.14828b804a1ebe804caddd1abb5d314d2be0a62a.sqlite
ica.25341683360a040721f587d069b95762b302f608.sqlite
rnd.5fc0960622bc4d3a5d366a0920dfd9b5ef578ced.sqlite

ica.ed49f6e7723fa2c15575c147566981beab6aba3e.sqlite
rnd.693940686b48985ab6ec8035a56575e0473e96de.sqlite
rnd.81273c47add77e98b3993e01cab8b2a836cd48b7.sqlite
sha.9316fc445533f9e58becf0d367d3a9404585ea86.sqlite
she.3464b427c15341dab8aaf6d89c8f79e5f24f9eae.sqlite
she.a0bd7c6accac92d54fb667bf32bcba4db4cd552f.sqlite
stake-pools.sqlite
```


<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number

ADP-1090


2858: Add (Instance of #ISSUE) re-write rule for bors comments r=rvl a=Anviking

- [x] Add simple re-write rules for bors-failure tags using annotations in Github issue titles

### Comments

This gives us a fairly easy way to close specific tickets in favor of a
more general one, without having to edit the corresponding bors-comments.

This is done by writing e.g. "(Instance of #2)" for issue #1, if you
want re-annotate failures from #1 as really being from #2.

**Example:** #2855 

<!-- Additional comments, links, or screenshots to attach, if any. -->

### Issue Number

<!-- Reference the Jira/GitHub issue that this PR relates to, and which requirements it tackles.
  Note: Jira issues of the form ADP- will be auto-linked. -->


Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
  • Loading branch information
iohk-bors[bot] and Anviking authored Aug 29, 2021
3 parents 9e71c1c + b077f8e + 6047e28 commit e43dec2
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 13 deletions.
26 changes: 17 additions & 9 deletions lib/core-integration/src/Test/Integration/Framework/DSL.hs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ import Numeric.Natural
( Natural )
import Prelude
import System.Command
( CmdOption (..), CmdResult, Stderr, Stdout (..), command )
( CmdOption (..), CmdResult, Exit (..), Stderr, Stdout (..), command )
import System.Directory
( doesPathExist )
import System.Exit
Expand Down Expand Up @@ -2507,7 +2507,7 @@ createWalletViaCLI
-> String
-> ResourceT m (ExitCode, String, Text)
createWalletViaCLI ctx args mnemonics secondFactor passphrase =
snd <$> allocate create (free)
snd <$> allocate create free
where
create = do
let portArgs =
Expand All @@ -2534,19 +2534,27 @@ createWalletViaCLI ctx args mnemonics secondFactor passphrase =
() <$ try @_ @SomeException (deleteWalletViaCLI @() ctx wid)

createWalletFromPublicKeyViaCLI
:: forall r s m.
( CmdResult r
, HasType (Port "wallet") s
:: forall s m.
( HasType (Port "wallet") s

, MonadIO m
)
=> s
-> [String]
-- ^ NAME, [--address-pool-gap INT], ACCOUNT_PUBLIC_KEY
-> m r
createWalletFromPublicKeyViaCLI ctx args = cardanoWalletCLI $
[ "wallet", "create", "from-public-key", "--port"
, show (ctx ^. typed @(Port "wallet"))] ++ args
-> ResourceT m (Exit, Stdout, Stderr)
createWalletFromPublicKeyViaCLI ctx args = snd <$> allocate create free
where
create =
cardanoWalletCLI $
[ "wallet", "create", "from-public-key", "--port"
, show (ctx ^. typed @(Port "wallet"))] ++ args

free (Exit (ExitFailure _), _, _) = return ()
free (Exit ExitSuccess, Stdout output, _) = do
w <- expectValidJSON (Proxy @ApiWallet) output
let wid = T.unpack $ w ^. walletId
() <$ try @_ @SomeException (deleteWalletViaCLI @() ctx wid)

deleteWalletViaCLI
:: forall r s m.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import Test.Integration.Framework.DSL
, icarusAddresses
, json
, minUTxOValue
, postByronWallet
, request
, restoreWalletFromPubKey
, selectCoins
Expand Down Expand Up @@ -117,7 +118,8 @@ spec = describe "BYRON_HW_WALLETS" $ do
"passphrase": #{fixturePassphrase},
"style": "icarus"
}|]
rInit <- request @ApiByronWallet ctx (Link.postWallet @'Byron) Default payldCrt

rInit <- postByronWallet ctx payldCrt
verify rInit
[ expectResponseCode HTTP.status201
, expectField (#balance . #available) (`shouldBe` Quantity 0)
Expand Down Expand Up @@ -294,8 +296,7 @@ spec = describe "BYRON_HW_WALLETS" $ do
"account_public_key": #{pubKey},
"address_pool_gap": #{addrPoolGap}
}|]
rRestore <- request @ApiByronWallet ctx (Link.postWallet @'Byron)
Default payloadRestore
rRestore <- postByronWallet ctx payloadRestore
expectResponseCode HTTP.status201 rRestore

let wPub = getFromResponse id rRestore
Expand Down Expand Up @@ -385,7 +386,7 @@ spec = describe "BYRON_HW_WALLETS" $ do
"passphrase": #{fixturePassphrase},
"style": "icarus"
}|]
r <- request @ApiByronWallet ctx (Link.postWallet @'Byron) Default payldCrt
r <- postByronWallet ctx payldCrt
expectResponseCode HTTP.status201 r

-- create from account public key
Expand Down
26 changes: 26 additions & 0 deletions scripts/bors-stats.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,11 @@ def failure_rate(n, nTot)
def fetch_comments_with_options(options)
comments = fetch_comments(target = options[:count], nil, options["force-refetch"]).sort_by { |x| event_date x }

# Fetching the title map here ourselves is suboptimal, but
# since we have on-disk caching is should be ok.
tm = fetch_gh_ticket_titlemap options
rewrite_tags(comments, tm)

if options[:search] then
comments = comments.filter {|x| x.bodyText.include? options[:search] }
end
Expand Down Expand Up @@ -638,6 +643,27 @@ def debug_trace(msg)
end
end

# Re-writes the tags of the provided comments list in-place,
# according to user-provided annotations in the issue titles.
#
# Annotations:
# "(Instance of #2) ..." - replace tag with #2
def rewrite_tags(comments, title_map)
rewrite_map = {}
title_map.each do |k,v|
new_tag = v["title"].scan(/^\(Instance of (#[\d]+)\)/).to_a.map { |x| x[0] }.first
if new_tag then
rewrite_map[k] = new_tag
end
end
comments.each do |c|
c.tags = c.tags.map do |t|
t2 = rewrite_map[t]
if t2 then t2 else t end
end
end
end

######################################################################
# ANSI color code helpers

Expand Down

0 comments on commit e43dec2

Please sign in to comment.