temporalio / sdk-go

Temporal Go SDK
https://docs.temporal.io/application-development?lang=go
MIT License
510 stars 204 forks source link

workflowcheck built on 1.23 does not find any findings #1627

Open aaomidi opened 1 week ago

aaomidi commented 1 week ago

We use this method for managing tools: https://marcofranssen.nl/manage-go-tools-via-go-modules

We've updated that go.mod to 1.23.0, and now when we run workflowcheck it has no findings. This is pretty easy to replicate, you can update the go.mod file in https://github.com/temporalio/sdk-go/blob/master/contrib/tools/workflowcheck/go.mod and see that workflowcheck isn't able to do proper analysis.

This code works properly up until 1.22.7.

aaomidi commented 1 week ago

I think this also shows a potential need for more test cases for the workflowcheck code, as updating the go sdk does not recognize that the analyzer itself actually stops working.

Quinn-With-Two-Ns commented 1 week ago

I tried updating the workflowcheck to 1.23.0 and see no test failure. Can you provided an example of something that was detected as none deterministic before but is no longer being properly detected?

Note the workflowcheck has no dependency on the Go SDK so an upgrade to the SDK would not impact workflowcheck

aaomidi commented 6 days ago
package workflows

import (
    "time"

    "go.temporal.io/sdk/workflow"
)

type SomeInterestingWorkInput struct{}

type SomeInterestingWorkOutput struct {
    HappenedAt int64
}

func SomeInterestingWork(ctx workflow.Context, input SomeInterestingWorkInput) (SomeInterestingWorkOutput, error) {
    // Do some interesting work here
    return SomeInterestingWorkOutput{
        HappenedAt: time.Now().Unix(),
    }, nil
}

If I install workflowcheck using go install go.temporal.io/sdk/contrib/tools/workflowcheck@latest, which I believe respects the go.mod file that workflowcheck has, it finds the time.Now() issue. If I clone the repo myself, change the go version, and do a go install . on it, then it stops being able to find them.