Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Split tests out into unit vs integration, and run each type in parallel #1083

Merged
merged 16 commits into from Nov 11, 2022
Merged

Split tests out into unit vs integration, and run each type in parallel #1083

merged 16 commits into from Nov 11, 2022

Conversation

lukemarsden
Copy link
Contributor

No description provided.

@wjam
Copy link
Contributor

wjam commented Nov 11, 2022

What's the expected gain from running unit tests separately but in parallel to the integration tests? I can see the advantage of running the unit tests before the integration tests, to give a quick sanity check before running the slower tests, but I'm not sure what the benefit if everything is going to be done in parallel.

I also have concerns that people are going to forget to tag their tests correctly. I'm not sure if there is a good way to do this in Go - may require playing with Bash or something? IMO, the best way to organise it would be the Rust/Java way where you can say all of the tests in this directory are unit tests and all of the tests in that directory are integration tests.

I also had a message pop up at some point complaining about concurrency limit in CircleCI? I'm not sure if this was a genuine problem, historical problem, or something else.

@lukemarsden
Copy link
Contributor Author

lukemarsden commented Nov 11, 2022 •
edited

Thanks for the review @wjam !

What's the expected gain from running unit tests separately but in parallel to the integration tests? I can see the advantage of running the unit tests before the integration tests, to give a quick sanity check before running the slower tests, but I'm not sure what the benefit if everything is going to be done in parallel.

a) it will be faster overall due to more parallelism
b) less to re-run on failure
c) you get feedback on your PR from CI faster
e) running go test locally gets you faster results, and you can run go test --tags integration at the end or just let the CI system take care of that for you

I also have concerns that people are going to forget to tag their tests correctly. I'm not sure if there is a good way to do this in Go - may require playing with Bash or something? IMO, the best way to organise it would be the Rust/Java way where you can say all of the tests in this directory are unit tests and all of the tests in that directory are integration tests.

The tags are integration or !integration - the latter isn't required, if developers forget to tag their test files, all that will happen is that the tests will run in both the unit and integration suites (I think). That's my read from https://mickey.dev/posts/go-build-tags-testing/ anyway, but please check my reasoning!

Hopefully the concurrency limit in CircleCI is something we can throw more money at them to fix, haven't seen that myself 🤷

@wjam
Copy link
Contributor

wjam commented Nov 11, 2022

But if the unit tests are so fast, is it worth the extra time needed to spin up more boxes to download/compile the same packages? How much code coverage do we get out of just the unit tests? (I you want to copy/paste the work I was doing on getting the coverage - https://github.com/filecoin-project/bacalhau/compare/coverage - feel free)

@lukemarsden
Copy link
Contributor Author

ah please land the coverage separately! That's awesome to have

The unittests aren't that fast, it's 45 seconds to a minute, I think there's some benefit here.

@lukemarsden lukemarsden merged commit 622f279 into main Nov 11, 2022
@lukemarsden lukemarsden deleted the unit-vs-integration-tests branch November 11, 2022 17:05
simonwo added a commit that referenced this pull request Nov 16, 2022
@philwinder @simonwo
[updatecli] Ops Bacalhau Dependency (#1167)
9327324 
# [updatecli] Ops Bacalhau Dependency


Bump ops clusters to "v0.3.12". This is an automated PR.



## Report

	Source:
		✔ [default] (githubrelease)


	Condition:

	Target:
		⚠ [development] Bump Dev(file)
		⚠ [production] Bump Dev(file)
		⚠ [staging] Bump Dev(file)




## Changelog

<details><summary>Click to expand</summary>

````

Release published on the 2022-11-16 14:05:57 +0000 UTC at the url https://github.com/filecoin-project/bacalhau/releases/tag/v0.3.12

## UX and core features
* Allow WASM jobs to have environment variables by @simonwo in #1026
* Don't overwrite files with hardlinks when downloading by @binocarlos in #985
* Support volumes with slashes for WASM jobs by @simonwo in #1027
* Fix running locally docs to use binary name by @iand in #1038
* Always use a random port of IPFS API server by @wjam in #1006
* Put WASM env vars in a consistent order by @wjam in #1051
* Add job timeout by @wdbaruni in #1061
* add --timeout cli option by @wdbaruni in #1065
* moving to beta by @aronchick in #1067
* 1015 fix CVE by @aronchick in #1019
* Load WASM import modules from IPFS (#880) by @lsp1138 in #1144
* Revert libp2p withdirectpeers by @lukemarsden in #1154
* More robust startup by @lukemarsden in #1159
* Pin results to Estuary after publishing to IPFS by @simonwo in #1002

## Developer experience
* Fix canary to look in correct results folder by @simonwo in #987
* Expose piggybacked publisher information by @wjam in #997
* Fetch build info from debug.BuildInfo struct by @wjam in #999
* Ensure temp files are cleaned up by @wjam in #1005
* Update dependencies and enable dependabot by @wjam in #1028
* Suppress logging output in tests by @iand in #1039
* Turn down some log messages that show up on every single test in CI by @lukemarsden in #1041
* Make GPU capacitymanager tests work when you have a GPU installed by @lukemarsden in #1042
* Remove test as it is a duplicate by @wjam in #1044
* Tweaks to make it easier to diagnose CI issues by @wjam in #1052
* First pass at removing Docker from tests which don't test it by @simonwo in #1054
* Don't run Docker tests on Windows or Mac by @wjam in #1066
* remove dead function GetSystemDirectory by @wdbaruni in #1072
* Associate the job id with both parts of the trace by @wjam in #1076
* have CircleCI notice the unittests.xml file by @lukemarsden in #1078
* Use Scenario-based tests to remove Docker and streamline test code by @simonwo in #1069
* Split tests out into unit vs integration, and run each type in parallel by @lukemarsden in #1083
* Clean up a directory related to testing by @wjam in #1128
* Expose test coverage by @wjam in #1123
* Use Noop executor in scenario-based tests by @simonwo in #1122
* Avoid flaky port collisions in tests by assign node ports up front by @simonwo in #1142
* Build canary against latest code by @simonwo in #1156
* Improve CI speed by @wjam in #1160
* Create CNAME by @aronchick in #1162
* Download Go on Windows directly by @wjam in #1164

## Dependency updates
* [updatecli] Ops Bacalhau Dependency by @philwinder in #984
* deps: updates wazero to 1.0.0-pre.3 by @codefromthecrypt in #989
* chore: Update Kubo to v0.16.0 by @Jorropo in #949
* Update Go version to 1.19 by @wjam in #1014
* Bump github.com/prometheus/client_golang from 1.13.1 to 1.14.0 by @dependabot in #1049
* Bump github.com/filecoin-project/go-jsonrpc from 0.1.8 to 0.1.9 by @dependabot in #1048
* Bump github.com/bmatcuk/doublestar/v4 from 4.3.2 to 4.4.0 by @dependabot in #1047
* Bump github.com/invopop/jsonschema from 0.6.0 to 0.7.0 by @dependabot in #1046
* Bump golang.org/x/net from 0.1.0 to 0.2.0 by @dependabot in #1055
* Bump github.com/filecoin-project/go-state-types from 0.9.8 to 0.9.9 by @dependabot in #1057
* Bump golang.org/x/mod from 0.6.0 to 0.7.0 by @dependabot in #1059
* Bump github.com/ipfs/go-ipfs-files from 0.1.1 to 0.2.0 by @dependabot in #1058
* Bump github.com/filecoin-project/go-address from 1.0.0 to 1.1.0 by @dependabot in #1056

## New Contributors
* @codefromthecrypt made their first contribution in #989
* @Jorropo made their first contribution in #949
* @iand made their first contribution in #1039
* @lsp1138 made their first contribution in #1144

**Full Changelog**: v0.3.11...v0.3.12


````

</details>

## Remark

This pull request was automatically created using
[Updatecli](https://www.updatecli.io).

Please report any issues with this tool
[here](https://github.com/updatecli/updatecli/issues/)


Co-authored-by: Simon Worthington <simon@register-dynamics.co.uk>
aronchick pushed a commit that referenced this pull request Dec 16, 2022
@philwinder @simonwo @aronchick
[updatecli] Ops Bacalhau Dependency (#1167)
6ab1acb 
# [updatecli] Ops Bacalhau Dependency


Bump ops clusters to "v0.3.12". This is an automated PR.



## Report

	Source:
		✔ [default] (githubrelease)


	Condition:

	Target:
		⚠ [development] Bump Dev(file)
		⚠ [production] Bump Dev(file)
		⚠ [staging] Bump Dev(file)




## Changelog

<details><summary>Click to expand</summary>

````

Release published on the 2022-11-16 14:05:57 +0000 UTC at the url https://github.com/filecoin-project/bacalhau/releases/tag/v0.3.12

## UX and core features
* Allow WASM jobs to have environment variables by @simonwo in #1026
* Don't overwrite files with hardlinks when downloading by @binocarlos in #985
* Support volumes with slashes for WASM jobs by @simonwo in #1027
* Fix running locally docs to use binary name by @iand in #1038
* Always use a random port of IPFS API server by @wjam in #1006
* Put WASM env vars in a consistent order by @wjam in #1051
* Add job timeout by @wdbaruni in #1061
* add --timeout cli option by @wdbaruni in #1065
* moving to beta by @aronchick in #1067
* 1015 fix CVE by @aronchick in #1019
* Load WASM import modules from IPFS (#880) by @lsp1138 in #1144
* Revert libp2p withdirectpeers by @lukemarsden in #1154
* More robust startup by @lukemarsden in #1159
* Pin results to Estuary after publishing to IPFS by @simonwo in #1002

## Developer experience
* Fix canary to look in correct results folder by @simonwo in #987
* Expose piggybacked publisher information by @wjam in #997
* Fetch build info from debug.BuildInfo struct by @wjam in #999
* Ensure temp files are cleaned up by @wjam in #1005
* Update dependencies and enable dependabot by @wjam in #1028
* Suppress logging output in tests by @iand in #1039
* Turn down some log messages that show up on every single test in CI by @lukemarsden in #1041
* Make GPU capacitymanager tests work when you have a GPU installed by @lukemarsden in #1042
* Remove test as it is a duplicate by @wjam in #1044
* Tweaks to make it easier to diagnose CI issues by @wjam in #1052
* First pass at removing Docker from tests which don't test it by @simonwo in #1054
* Don't run Docker tests on Windows or Mac by @wjam in #1066
* remove dead function GetSystemDirectory by @wdbaruni in #1072
* Associate the job id with both parts of the trace by @wjam in #1076
* have CircleCI notice the unittests.xml file by @lukemarsden in #1078
* Use Scenario-based tests to remove Docker and streamline test code by @simonwo in #1069
* Split tests out into unit vs integration, and run each type in parallel by @lukemarsden in #1083
* Clean up a directory related to testing by @wjam in #1128
* Expose test coverage by @wjam in #1123
* Use Noop executor in scenario-based tests by @simonwo in #1122
* Avoid flaky port collisions in tests by assign node ports up front by @simonwo in #1142
* Build canary against latest code by @simonwo in #1156
* Improve CI speed by @wjam in #1160
* Create CNAME by @aronchick in #1162
* Download Go on Windows directly by @wjam in #1164

## Dependency updates
* [updatecli] Ops Bacalhau Dependency by @philwinder in #984
* deps: updates wazero to 1.0.0-pre.3 by @codefromthecrypt in #989
* chore: Update Kubo to v0.16.0 by @Jorropo in #949
* Update Go version to 1.19 by @wjam in #1014
* Bump github.com/prometheus/client_golang from 1.13.1 to 1.14.0 by @dependabot in #1049
* Bump github.com/filecoin-project/go-jsonrpc from 0.1.8 to 0.1.9 by @dependabot in #1048
* Bump github.com/bmatcuk/doublestar/v4 from 4.3.2 to 4.4.0 by @dependabot in #1047
* Bump github.com/invopop/jsonschema from 0.6.0 to 0.7.0 by @dependabot in #1046
* Bump golang.org/x/net from 0.1.0 to 0.2.0 by @dependabot in #1055
* Bump github.com/filecoin-project/go-state-types from 0.9.8 to 0.9.9 by @dependabot in #1057
* Bump golang.org/x/mod from 0.6.0 to 0.7.0 by @dependabot in #1059
* Bump github.com/ipfs/go-ipfs-files from 0.1.1 to 0.2.0 by @dependabot in #1058
* Bump github.com/filecoin-project/go-address from 1.0.0 to 1.1.0 by @dependabot in #1056

## New Contributors
* @codefromthecrypt made their first contribution in #989
* @Jorropo made their first contribution in #949
* @iand made their first contribution in #1039
* @lsp1138 made their first contribution in #1144

**Full Changelog**: v0.3.11...v0.3.12


````

</details>

## Remark

This pull request was automatically created using
[Updatecli](https://www.updatecli.io).

Please report any issues with this tool
[here](https://github.com/updatecli/updatecli/issues/)


Co-authored-by: Simon Worthington <simon@register-dynamics.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lukemarsden @wjam