Add more integration tests for the runner #175
Conversation
Could you explain this a bit more? Do you mean that the sourcing is a piece that we particularly want to test? |
sampart
left a comment
sampart
left a comment
There was a problem hiding this comment.
Thanks for this. Just one question.
| - name: Run init | ||
| run: | | ||
| runner/dist/codeql-runner-win.exe init --repository $Env:GITHUB_REPOSITORY --languages javascript --github-url $Env:GITHUB_SERVER_URL --github-auth ${{ github.token }} |
There was a problem hiding this comment.
Is there a reason why you don't pass the config file here, but you do for the Linux test?
There was a problem hiding this comment.
Most of the other tests further down also don't use it. I imagine there's a reason why it's in one but not others, but I just wanted to check.
There was a problem hiding this comment.
That's unintentional. I think it's because when I started I was going to upload the results so I wanted to config to mirror our existing codeql.yml workflow, but then I switched mid-way to not uploading.
Thinking about it, maybe the best compromise is to leave it as it is so the ubuntu javascript test has the config-file parameter but not the rest. Not sure it matters too much. It'll make that one job a bit slower as we'll be running more queries, but it means we're testing the config file parsing for the runner. Testing the parsing on multiple operating systems doesn't really add much. I'll add a comment explaining this.
Yes. I mean that that is a bit we get a lot of benefit from testing. The rest of it is either simpler or the same as when running on actions. |
Moves the existing test of the runner to the
integration-testing.ymlworkflow, and adds more tests, including:This does come to quite a lot of workflows. If it's too much we could cut down on some of the operating systems for javascript analysis, and the autobuild test. Sourcing the environment variables on all operating systems is the bit that likely gets us the most benefit. But we can also just add them all and cut down in the future.
Merge / deployment checklist