nkanderson / ECE585-llc

Simulation of a last level cache (LLC) for ECE 585 final project
1 stars 0 forks source link

Add main program loop #35

Closed nkanderson closed 1 week ago

nkanderson commented 1 week ago

Adds a basic program loop and event handler to process each operation from the trace file. Also configures module-level instances of the logger and args, so they can be imported and used throughout the program.

FYI, I'm working on a basic test structure for handle_event, but I wanted to get this up for review because the tests are going to be trivial at this point since the function doesn't really do anything yet. Getting a little stuck on our test directory structure and module imports, so I'm trying to work through that considering that we also discussed wanting to rearrange this directory structure anyways.

Closes #18

nkanderson commented 1 week ago

@reecewayt @mdhardenburgh @d-engler this should be ready for review now. I recommend looking at it commit by commit, though the first 2 commits are related. Since I got the Dockerfile and tests directory stuff sorted out, I was able to add a check in github actions that runs our unittests 🎉

If you want to see the output, clock on "show all checks" (which is "hide all checks" in the image below), then the details for the unittest one: Screenshot 2024-11-12 at 8 30 32 PM

reecewayt commented 1 week ago

@nkanderson So Github actions will run all unittests automatically, am I understanding that correctly? The docker stuff I'm still not familiar with but it looks like we can now run unittest with docker run --rm -it llcsim:dev once we've built the Docker test and development image?

Other than these questions everything looks good to me and I like how you've re-organized the project config stuff.

nkanderson commented 1 week ago

So Github actions will run all unittests automatically, am I understanding that correctly?

Yep, just like the linter, it will run on any push / merge to main, plus any PRs targeting main. The actions tab shows all the workflow runs and which branch they came from, etc.

it looks like we can now run unittest with docker run --rm -it llcsim:dev once we've built the Docker test and development image

Exactly, if you run docker build --target test -t llcsim:dev . you should get the updated image built, and then that docker run command should run the tests automatically (you can still run docker run --rm -it llcsim:dev bash if you want to shell into the container, but usually you'd want to use docker compose for that in order to make sure you have the volumes mapped so your changes persist). You can also run the tests in the container. The compose file now has a volume mapped for the tests directory, so running something like python -m unittest or python -m unittest tests.utils.test_cache_logger should work from the base directory you're dropped into when starting up the container via docker compose.

FYI, I did have to run docker compose build --no-cache to clear out a stale image. I'm not sure if that will be a problem for anyone else or not, since I've been doing a lot of testing and rebuilding of images.

It's possible we'll want to tweak the actual python -m unittest command that's being run in the test image and in CI if we adjust how are tests directory is structured, but I think this is a safe default to begin with.

nkanderson commented 1 week ago

Discussed with @reecewayt, going ahead with the merge