ci: add backwards compatibility tests #23
Conversation
cmd/root.go
Outdated
| _ = rootCmd.Execute() | ||
| err = rootCmd.Execute() | ||
| if err != nil { | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
I have a problem with this code.
There should be no os.Exit in code outside main.go.
I would like to suggest you to update the signature of execute to return an error. And to have this if err != nil os.Exit(1) in main.go
It's not related to this PR, the problem was already there. But I'm discovering it now, so I report it now
.github/workflows/back-compat-pr.yml
Outdated
| with: | ||
| ref: main | ||
| - name: build main | ||
| run: | | ||
| go build -o omm_main | ||
| cp omm_main /var/tmp | ||
| - name: Run last version | ||
| run: | | ||
| /var/tmp/omm_main --db-path=omm-test.db 'test: a task from main' |
There was a problem hiding this comment.
Cannot this be covered by a test?
Could what I suggested to fix some issues that led you doing this here?
There was a problem hiding this comment.
This is for ensuring any changes to the data persistence side of things (database migrations, handling of errors) do not mess anything up for already running instances of omm. Specifically, for ensuring that a change doesn't break for folks who already have a omm.db file out there.
I guess this may be tested via code tests (would need simulating real world conditions in tests); running the real thing in CI seemed like a good first step.
There was a problem hiding this comment.
Eg. for cases like this one.
.github/workflows/back-compat.yml
Outdated
| - name: Run last version | ||
| run: | | ||
| /var/tmp/omm_prev --db-path=omm-test.db 'test: a task from previous commit' |
There was a problem hiding this comment.
Are you sure you will be able to merge/release if a bug occur ?
Maybe because a library changed, or you bumped go version.
I mean if code is KO, this will fail, so you won't be able to merge?
There was a problem hiding this comment.
I didn't fully understand. Could you describe the scenario a bit more?
There was a problem hiding this comment.
You are adding something to the workflow.
So to something that may prevent you to do errors
Now imagine you merged something, and because of something outside your code. It could be the go version, an external package, a tool… your tests no longer pass, even if they did before.
So now your tests are failing.
So you provide a fix
You push
The workflow is run
The code that check if it's a regression, the code you added with the PR about the workflow, will fail because tests are failing.
So now the workflow is failing, won't it prevent you to merge the fix ?
No description provided.