@acfoltzer

Before we merge, we'll need to add some tests for the new interface.

I had a look, but as you said, it was a bit tricky to figure out where and how to add the tests 😅

I can give you guidance on navigating our test suite.

I'm happy to take a stab at it, if you give me some details... Could you let me know where the right place in the repo would be for these tests (Is it lucet-runtime-tests, lucet-runtime/tests or should I create a new test folder under lucet-runtime-internals?) Any additional guidelines would be appreciated as well.

It might be a little bit before I can get to doing that, so if merging the PR quickly is important

If this is a matter of a couple of weeks, I'm happy to wait as well 😃

If this is a matter of a couple of weeks, I'm happy to wait as well

A couple weeks sounds about right. I'd also like to use this as an opportunity to write some better docs about how our test suite is laid out. We have the start of docs on the wiki but that page is not sufficient to answer your questions.

@shravanrn sorry for the delay on this. I've added more detail to the wiki page that hopefully helps answer your questions.

To make the advice there specific to this use case, I would put unit tests in /lucet-runtime/lucet-runtime-internals/src/region/mmap.rs or a new child module, depending on size; integration tests should probably go in /lucet-runtime/tests.

Another idea that could help get a lot of test coverage quickly without too much new typing would be to create a wrapper type, something like AlignedMmapRegion, that would create an underlying MmapRegion with a particular alignment. Then, where you see the test macros instantiated with MmapRegion, you could add another instantiation with AlignedMmapRegion. I think it'd still be worth writing some tests that are specific to the alignment APIs, though.

That's great! Thanks for the info. This next week unfortunately is pretty busy, but I should be able to get some tests written up the following week.

Hi @acfoltzer ,
Sorry for the delay here. I got pulled away on some other stuff and have finally resumed the lucet changes. I have rebased and added some tests. Does this look ok?

@acfoltzer @pchickey - just following up about this

Have made requested changes.