gremlins icon indicating copy to clipboard operation
gremlins copied to clipboard

Can't run gremlins in drives different from temp directory

Open Thunder33345 opened this issue 1 year ago • 8 comments

🐞Bug Report

Bug Description

When the system temp directory's drive is not the same as the current project's drive, gremlins unleash fails to run

To Reproduce

Steps to reproduce the behavior:

  1. Create a project on another drive that is different from the one in the TEMP(C:) variable
  2. Execute gremlins unleash on said project(D:) that happens to be in another drive
  3. See terminal error with panic: error, this is temporary
Console Log
Starting...
Gathering coverage... go: no module dependencies to download
done in 1.4549877s
Executing mutation testing on covered mutants...
panic: error, this is temporary

goroutine 8 [running]:
github.com/go-gremlins/gremlins/pkg/mutator.(*Mutator).executeTests(0xc000156480, {0x739190, 0xc00005a680})
        /go/pkg/mod/github.com/go-gremlins/[email protected]/pkg/mutator/mutator.go:208 +0x88e
github.com/go-gremlins/gremlins/pkg/mutator.(*Mutator).Run(0xc000156480, {0x739190, 0xc00005a680})
        /go/pkg/mod/github.com/go-gremlins/[email protected]/pkg/mutator/mutator.go:147 +0xfb
github.com/go-gremlins/gremlins/cmd.run({0x739190, 0xc00005a680}, {0xc00001f700, 0x35}, {0x6936d0, 0x1})
        /go/pkg/mod/github.com/go-gremlins/[email protected]/cmd/unleash.go:155 +0x46c
github.com/go-gremlins/gremlins/cmd.runUnleash.func1.1({0x739190?, 0xc00005a680?})
        /go/pkg/mod/github.com/go-gremlins/[email protected]/cmd/unleash.go:104 +0x78
github.com/go-gremlins/gremlins/cmd.runWithCancel({0x739190?, 0xc00005a4c0}, 0x0?, 0xc00005a640, 0xc00004fa60)
        /go/pkg/mod/github.com/go-gremlins/[email protected]/cmd/unleash.go:128 +0xd2
created by github.com/go-gremlins/gremlins/cmd.runUnleash.func1
        /go/pkg/mod/github.com/go-gremlins/[email protected]/cmd/unleash.go:103 +0x43c

Found behaviour

Received an error panic: error, this is temporary

Expected behaviour

Gremlins should be able to work with different drives

Workaround

Set your TEMP variable to one which matches your project directory's drive letter. In this case set tmp=D:/tmp will do the trick

Operating System

  • OS: Windows 10
  • Version Gremlins 0.2.2

Additional Context

I have also created a custom build which includes error message into the panic. That gave me some more clue on what actually went wrong, I found the actual error to be The system cannot move the file to a different disk drive. and let me discover a workaround for it.

Console Log
Starting...
Gathering coverage... go: no module dependencies to download
done in 1.4664947s
Executing mutation testing on covered mutants...
panic: error, this is temporary: link D:\...\LICENSE C:\...\Temp\gremlins-2167308290\wd-1166897977\LICENSE: The system cannot move the file to a different disk drive.
While it's true I am using a custom build here for debugging, I have verified that both the error and workaround can be replicated on the official build.

Thunder33345 avatar Aug 15 '22 16:08 Thunder33345

Hi @Thunder33345, thanks for the bug report. We faced the same issue when we worked on making Gremlins run in a Docker container. So, we have a clue on how to fix this, but we have to do it properly. Since there is a workaround and the next minor is around the corner, I would plan this fix for v0.3 rather than a hotfix version on v0.2. Also, it'd be wise to first merge PR #127.

@giose86 I think we have two options here:

  1. Completely abandon the hardlink approach and just make copies
  2. Provide a flag to force "docker mode" even if not in a Docker container (changing name)

I think the first solution is the proper one, but I'd first do some benchmarks to see if and how much performance we loose. If it is too much, we can go with solution 2.

Also, let's not forget to fix that stupid temporary panic I left in the code! (sorry for that)

k3rn31 avatar Aug 15 '22 17:08 k3rn31

@k3rn31 let me try the first one solution by collecting some performance data. The second one is already there, more or less, if I go too long we can implement the latter by adding the flag you said :)

giose86 avatar Aug 15 '22 18:08 giose86

@giose86 if first approach works, we can optimise the mutations application by removing the removal of the file (to do after #127 merge).

k3rn31 avatar Aug 15 '22 18:08 k3rn31

Hello there, just checking this out after seeing it showcased in discord!

What about a fallback strategy? When it detect that it's not possible to make a hard link, just fallback to copying instead.

Or have a way to specify temp directory for gremlins to use (so i could just specify /tmp/) and it would make it, and clean up after use.

Thunder33345 avatar Aug 15 '22 19:08 Thunder33345

I think the fallback is a great idea. Let's first see if switching to copying has impact on the performance. If the timings are comparable, it would be simpler to just copy. If, instead, the loss of performance is significant, both falling back or specifying the temp dir are excellent solutions. We can even implement both of them.

k3rn31 avatar Aug 16 '22 03:08 k3rn31

Since I want to release v0.3 so that it can be tested, I'm moving this one to the next minor (v0.4). If it doesn't take too much time, we'll release a v0.3.1. Cheers.

k3rn31 avatar Aug 17 '22 16:08 k3rn31

Hi guys, this is what i got by replacing the hard link with the copy:

PC info goos: linux goarch: amd64 cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz

Benchstat name old time/op new time/op delta DealerGet-12 9.33s ± 0% 17.75s ± 0% ~ (p=1.000 n=1+1)

Running gremlins on gremlins itself:

Mutation testing completed in 20 seconds 108 milliseconds Killed: 116, Lived: 22, Not covered: 9 Timed out: 3, Not viable: 2 Test efficacy: 84.06% Mutant coverage: 93.88%

i think we can safely remove the hard link without loosing so much performance, what you think about it?

giose86 avatar Sep 02 '22 14:09 giose86

I’d say go for it. The performance difference is negligible and there will be less code to maintain and reason about.

k3rn31 avatar Sep 02 '22 15:09 k3rn31