Split expect tests into separate circleci group

[winder]

Summary

Attempt to split expect tests into a 3rd category.

This changes the behavior of e2e_go_tests.sh. Previously it would run the e2e_go tests AND the EXPECT tests. Now it runs one or the other.

Test Plan

CI

[codecov-commenter]

Codecov Report

Merging #2792 (85d8860) into master (bd5a000) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2792      +/-   ##
==========================================
- Coverage   47.08%   47.07%   -0.02%     
==========================================
  Files         349      349              
  Lines       56401    56401              
==========================================
- Hits        26558    26550       -8     
- Misses      26865    26871       +6     
- Partials     2978     2980       +2     

Impacted Files	Coverage Δ
ledger/blockqueue.go	81.03% <0.00%> (-4.03%)	⬇️
catchup/peerSelector.go	98.95% <0.00%> (-1.05%)	⬇️
network/requestTracker.go	70.25% <0.00%> (-0.87%)	⬇️
ledger/acctupdates.go	62.13% <0.00%> (-0.42%)	⬇️
network/wsNetwork.go	60.90% <0.00%> (-0.19%)	⬇️
data/transactions/verify/txn.go	45.08% <0.00%> (ø)
network/wsPeer.go	75.20% <0.00%> (+0.83%)	⬆️
crypto/merkletrie/node.go	93.48% <0.00%> (+1.86%)	⬆️
crypto/merkletrie/trie.go	68.61% <0.00%> (+2.18%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd5a000...85d8860. Read the comment docs.

[tsachiherman]

@winder I'm ok with this - but please fix the review dog errors.

[algorandskiy]

LGTM, just remove from goal_expect_test.go

	t.Skip("goal expect test are disabled due to flakiness")

[algorandskiy]

Let's see if it works

[algobarb]

Can we also test that nightly tests will work as expected too? Just in case there are expect tests that are disabled in -short that may still be impacted. To test, you can remove all slack references and replace /hotfix\/.*/ with/pull\/[0-9]+/

[winder]

Can we also test that nightly tests will work as expected too? Just in case there are expect tests that are disabled in -short that may still be impacted. To test, you can remove all slack references and replace /hotfix\/.*/ with/pull\/[0-9]+/

All of the expect tests go through the same test function, and it doesn't use Short, so this wouldn't change anything.

I thought we didn't have any tests using that option in e2e-go, but there are some:

~/go/src/github.com/algorand/go-algorand/test/e2e-go$ rg Short
features/participation/overlappingParticipationKeys_test.go
64:	// ShortParticipationKeys template has LastPartKeyRound=8
66:	fixture.SetupNoStart(t, filepath.Join("nettemplates", "ShortParticipationKeys.json"))

features/participation/participationRewards_test.go
143:	if testing.Short() {

features/teal/compile_test.go
34:	if testing.Short() {

features/catchup/catchpointCatchup_test.go
85:	if testing.Short() {

features/catchup/basicCatchup_test.go
37:	if testing.Short() {
105:	if testing.Short() {
205:	if testing.Short() {

features/participation/onlineOfflineParticipation_test.go
111:	if testing.Short() {

features/partitionRecovery/partitionRecovery_test.go
36:	if testing.Short() {
82:	if testing.Short() {
104:	if testing.Short() {
167:	if testing.Short() {
220:	if testing.Short() {

features/transactions/sendReceive_test.go
48:	if testing.Short() {
58:	if testing.Short() {

features/transactions/asset_test.go
192:	if testing.Short() {
978:	if testing.Short() {
985:	// Setting the testShorterLookback parameters derived from ConsensusCurrentVersion
999:	a, fixture, client, account0 := setupTestAndNetwork(t, "TwoNodes50EachTestShorterLookback.json", configurableConsensus)

[algorandskiy]

Looks like it is passing so it is working, voting for merging the PR in.

[algobarb]

Looks good. Tests passed and if they are separated out, then like you said, it shouldn't affect the nightly integration tests in terms of CPU consumption.

[cce]

This seems to have made the overall build time slower — due to splitting these out from the "integration" job (which was sharded by 4 nodes) into a new unsharded job. Also, I am confused, what is left in the old "integration" job now?

[cce]

The result_subdir here doesn't match

[winder]

@cce we have 3 main types of integration tests:

1. "scripts", also referred to as "e2e subs", are the bash scripts executed (mostly in parallel) by e2e_client_runner.py
2. "go tests", referred to here as "integration", are integration tests written in go. We have a number of fixtures in test/e2e-go. Basically this is go test in that directory.
3. "expect tests" are a subset of the "go tests". There is a fixture in the "go tests" to execute tests written with expect.

The expect tests take a long time to run and have been disabled for a while. Maybe the expect tests don't shard properly because they run through a single fixture?

[cce]

I think these tests were running as part of the "integration" job before, so this upgrades the resource_class from "medium" to "large" — we might want to go back down

[cce]

I think this means that whether or not RUN_EXPECT is set to FALSE or TRUE (and it is set to FALSE by default at the top of this file) we will only ever run the expect tests.

[cce]

Opened #2863