Contributor

This is the only test case I continue to see failing on appveyor. Weird because it is obviously a remove_dir_all bug, so I'd expect it to be fixed.

This is just remove the fixme and making the error handling better. I don't know what the real fix is.

cc @Diggsey

Contributor

Isn't the assert still going to fail?

Contributor Author

Yes it will.

Contributor

@brson I think the change_dir might be the issue? (windows prevents the current directory of any program from being deleted iirc)

Also, separately, won't using change_dir like that have problems when tests are run in parallel?

We should be able to change the current directory of the executed program, without touching our own current directory.

Contributor Author

@Diggsey change_dir could be at fault here. That function does restore the original directory after the closure exits though.

The clitools::setup function holds a lock for the duration of the test because of the change_dir issue. All the cli tests are mostly serialized.

Oh, this patch is bogus for checking the error - TempDir is supposed to delete that directory itself. The remove_dir call is a second attempt to delete. But that reminds me that tempdir got a patch to use remove_dir_all, though it hasn't been released yet. Upgrading tempdir might fix this bug.

Contributor Author

This patch is bogus.