robur-coop / albatross

Albatross: orchestrate and manage MirageOS unikernels with Solo5
ISC License
141 stars 17 forks source link

naming, and some tests #111

Closed hannesm closed 2 years ago

hannesm commented 2 years ago

This looks overall promising. Left to do:

hannesm commented 2 years ago

revised API again (of vmm_trie), added some tests (now discovered that all/collect/fold can't work well due to information lost at insert (a Name.t with an empty name (= None) "foo:bar:" is encoded the same as "foo:bar" in the trie). need to think about this a bit more, esp. what is in the trie and how to expose it... eventually store a '(a * bool) option with the bool indicating whether it is a path or a name...

hannesm commented 2 years ago

This is finally ready, including test cases for name/vmm_trie and vmm_resources (in addition to the asn.1 decoding tests to ensure older clients are supported by newer servers).

@reynir if you could have a brief look, that'd be great (I suggest https://github.com/roburio/albatross/pull/111/files?w=1 to ignore whitestpace changes). I added the bisect_ppx runes to src/dune to measure the test coverage, esp. in vmm_trie and vmm_resources (and am satisfied with the result). I also suggest a squash & merge since intermediate states are not important.

hannesm commented 2 years ago

I committed your suggestions, and added ae5606d

reynir commented 2 years ago

Cool! I think this looks very good now. Thanks!

hannesm commented 2 years ago

For future reference: this migration worked smoothly, although the block device naming was broken (robur.tlstunnel vs robur:tlstunnel) and I had to move them manually.