Add integration test boilerplate

[xxchan]

Currently, the code of setting up integration tests https://github.com/beyondstorage/go-integration-test is copied, and problematic. We should fix the problems and maybe we can add a boilerplate here.

• typo exmaple here https://github.com/beyondstorage/go-service-fs/blob/master/tests/README.md#L8
• Makefile.env should be included like https://github.com/beyondstorage/go-service-s3/blob/master/Makefile#L3 but is missing in https://github.com/beyondstorage/go-service-fs/blob/master/Makefile#L1-L3

[xxchan]

export is redundant if we use Makefile.env https://github.com/beyondstorage/go-service-fs/blob/master/tests/README.md#L22

GitHub

beyondstorage/go-service-fs

https://beyondstorage.io/docs/go-storage/services/fs - beyondstorage/go-service-fs

[Xuanwo]

export is redundant if we use Makefile.env https://github.com/beyondstorage/go-service-fs/blob/master/tests/README.md#L22

We can't use Makefile.env in CI, so we need to export those environments. And what you said is correct, we don't need to use export here, instead, we need to declare them in github action files like:

- name: Test
  env:
    STORAGE_FS_INTEGRATION_TEST: ${{ secrets.STORAGE_INTEGRATION_TEST }}
  run: make integration_test

Maybe we need to make the statement more clear.

GitHub

beyondstorage/go-service-fs

https://beyondstorage.io/docs/go-storage/services/fs - beyondstorage/go-service-fs

[xxchan]

(Optional) clean up after the test. But this may rely on the implementation of Delete

[xxchan]

Consider when and how to trigger workflow. We hope to require PRs to pass integration tests before merged but it is dangerous to allow arbitrary code (from forked repo) to run in CI.

We can

• trigger tests manually by reviewers. e.g., commenting /run-all-tests and let a bot trigger the tests. This seems to be common practice, but may need extra infra?
• or use pull_request_target (for secrets access) and limit type to labeled. Then the workflow is triggered when a reviewer add any labels (we can add a label like safe to test).

We can't simply use pull_request, since it cannot access secrets.

[xxchan]

What about failing the test when STORAGE_*_INTEGRATION_TEST is not on?

[Xuanwo]

What about failing the test when STORAGE_*_INTEGRATION_TEST is not on?

LGTM

[Xuanwo]

Current design led to new push can't trigger tests: beyondstorage/go-service-ipfs#7 (comment)

Is it possible to trigger tests when there are new commits and has been labeled as safe to test? Or it's OK to run tests when there are no changes to Makefile and workflow files?

[Xuanwo]

Maybe we can implement that logic in beyondstorage/go-community#9.

• Use pull_request_target instead of pull_request
• Remove the labeled restriction
• Implement GitHub action to check whether we need to run integration tests or not
  • If services are hosted in actions instead of our infra: OK
  • If the author is our reviewer or above: OK
  • If the PR doesn't change workflow or Makefile: OK
  • If the PR received a /safe or /integration-tests from a reviewer or above: OK
  • ...

[xxchan]

Current design led to new push can't trigger tests

I think this is intended. Otherwise an attacker can push an attack after being labeled.

"safe to test" is valid only once, just like a "/run-all-tests" command

[Xuanwo]

"safe to test" is valid only once, just like a "/run-all-tests" command

But untag & tag everytime adds an extra burden for reviewers, any better ideas?

[Xuanwo]

Should be fixed.