dagstore icon indicating copy to clipboard operation
dagstore copied to clipboard

feat: add unified-ci

Open rvagg opened this issue 3 years ago • 1 comments

Since I need to work on a proposal for a change here (based on discussion @ https://github.com/filecoin-project/boost/pull/715) I thought I'd get familiar with the codebase by attempting a conversion to unified-ci. Maybe there are objections to doing this here though?

Still needs registering in protocol/.github so we get automatic syncs.

Main changes:

  • Tests with 1.18 and 1.19 across Linux, macOS and Windows
  • Bumps go.mod to 1.18
  • Remove CircleCI config
  • Remove use of ioutil
  • Fix staticcheck errors - mainly just missing err checks in tests but it also found an interesting one in registry_test.go where a URL is modified with a noop, I'm not sure it's changed any test results though!

The most interesting bit is trying to get Windows compatibility. In the end I skipped a few of the tests but the main problem seems like a real issue worth fixing at some point—there's no way to properly clean up resources on a Dagstore Close. It's very easy to have open file handles and Windows is a bit more strict about removing directories (temp for the tests) when there's open file handles. In particular, the mounts, as they go through Upgrader, have the .completed file left open in many cases and there's no easy way to get it closed before end of test. Ideally a Dagstore Close would handle this but it's not straightforward enough for me to wire all that up in this PR. The Context cancellation test presents a whole other set of issues around resource cleanup, you have open file handles stuck in the channel queue system that just gets dropped on the floor when the context is cancelled. We probably need to track active mounts internally and close them out properly at the end and also figure out how to wire the context through the necessary paths to also close them out on cancellation, although I'm not sure about that because contexts are handled inconsistently through the various API calls so it might be a challenge.

rvagg avatar Aug 27 '22 00:08 rvagg

Restored circle config since I don't have access to turn it off. I think this is good to go. Anyone want to :thumbsup:?

rvagg avatar Aug 30 '22 11:08 rvagg