Skip to content

chore: delete legacy functional-tests directory (STR-2085)#1581

Merged
storopoli merged 5 commits intomainfrom
chore/delete-legacy-functional-tests
Apr 13, 2026
Merged

chore: delete legacy functional-tests directory (STR-2085)#1581
storopoli merged 5 commits intomainfrom
chore/delete-legacy-functional-tests

Conversation

@voidash
Copy link
Copy Markdown
Contributor

@voidash voidash commented Apr 7, 2026

Summary

Deletes the entire functional-tests/ directory. All tests have been migrated to functional-tests-new/ or their invariants tracked in JIRA/Notion.

What's deleted

What's added

  • test_sync_genesis.py verifies chain reaches genesis and tip advances consistently
  • Fix in strata_seq_fullnode.py: was passing GenesisL1View object instead of int to create_node()

CI changes

  • Delete .github/workflows/functional.yml (legacy test workflow)
  • Update test-coverage.yml to remove legacy test job
  • Update main-eest.yml to use functional-tests-new/
  • Update dependabot.yml, CODEOWNERS

Test plan

  • test_sync_genesis passes locally
  • CI functional-tests-new workflow passes
  • No stale references to functional-tests/ in CI or docs

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Commit: 928cb1e

SP1 Execution Results

program cycles success
EVM EE STF 1,318,471
Checkpoint 4,720
Checkpoint New 883,564

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.47%. Comparing base (6dcc0fb) to head (f9f4bc9).
⚠️ Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (6dcc0fb) and HEAD (f9f4bc9). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (6dcc0fb) HEAD (f9f4bc9)
functional 1 0
@@            Coverage Diff             @@
##             main    #1581      +/-   ##
==========================================
- Coverage   72.35%   62.47%   -9.89%     
==========================================
  Files         687      685       -2     
  Lines       69959    69863      -96     
==========================================
- Hits        50622    43644    -6978     
- Misses      19337    26219    +6882     
Flag Coverage Δ
functional ?
unit 62.47% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 189 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also rename functional-tests-new to simply functional-tests in this PR as well? (I think so)

@voidash voidash marked this pull request as ready for review April 8, 2026 12:21
@voidash voidash requested review from a team as code owners April 8, 2026 12:21
@voidash voidash force-pushed the chore/delete-legacy-functional-tests branch from 0a04001 to f3130d6 Compare April 8, 2026 12:40
Copy link
Copy Markdown
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff also shows that you've deleted a bunch of functional-tests-new stuff instead of renaming? Is that correct. See screenshot below:

Image

Comment thread .github/dependabot.yml Outdated
commit-message:
prefix: "chore(deps)"
include: "scope"
# Legacy functional-tests/ removed — see functional-tests/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep this, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored now. The uv block's directory path /functional-tests/ is identical after the rename, so it covers the new framework without any changes.

@storopoli
Copy link
Copy Markdown
Member

NOTE to myself. After this PR is merged we need to update the merge queue requirements because of the renaming
CleanShot 2026-04-08 at 12 24 52@2x

Comment thread .dockerignore
@voidash
Copy link
Copy Markdown
Contributor Author

voidash commented Apr 9, 2026

The diff also shows that you've deleted a bunch of functional-tests-new stuff instead of renaming? Is that correct. See screenshot below:

final tree should be correct. This is because how commits are structured

storopoli
storopoli previously approved these changes Apr 9, 2026
Copy link
Copy Markdown
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK efe9f15

@storopoli storopoli enabled auto-merge April 9, 2026 13:15
@storopoli storopoli requested a review from purusang April 9, 2026 13:15
@delbonis
Copy link
Copy Markdown
Contributor

delbonis commented Apr 9, 2026

some merge conflicts, will review more thoroughly shortly

@voidash voidash force-pushed the chore/delete-legacy-functional-tests branch from efe9f15 to f81f15c Compare April 13, 2026 08:45
prajwolrg
prajwolrg previously approved these changes Apr 13, 2026
storopoli
storopoli previously approved these changes Apr 13, 2026
Copy link
Copy Markdown
Member

@storopoli storopoli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK f81f15c

@storopoli
Copy link
Copy Markdown
Member

@delbonis ping, should be ready to merge now.

delbonis
delbonis previously approved these changes Apr 13, 2026
@delbonis
Copy link
Copy Markdown
Contributor

delbonis commented Apr 13, 2026

MERGE CONFLICTS NOOO

@storopoli
Copy link
Copy Markdown
Member

I'll fix them and force-merge

voidash added 3 commits April 13, 2026 15:19
Remove the entire functional-tests/ directory. All valuable tests have been
migrated to functional-tests-new/ or their invariants tracked in JIRA/Notion.

Sync tests: added test_sync_genesis to functional-tests-new. Fullnode lag
restart test deferred — fullnode sync not yet implemented in new strata binary
(tracked in STR-2091).

Also:
- Fix bug in strata_seq_fullnode.py: pass genesis_l1.blk.height (int) not
  GenesisL1View object to create_node()
- Delete .github/workflows/functional.yml (legacy test workflow)
- Update test-coverage.yml, main-eest.yml, dependabot.yml, CODEOWNERS
- Update AGENTS.md, README.md, CONTRIBUTING.md references
Now that the legacy functional-tests/ directory is deleted, drop the
-new suffix. Updates all references across CI workflows, justfile,
CODEOWNERS, Dockerfile, docs, and .dockerignore.
Test run data directory contains generated TOML files that should not
be linted.
voidash and others added 2 commits April 13, 2026 15:19
…085)

- CODEOWNERS: drop fn_*.py and constants.py patterns that match nothing
  in the new layout.
- dependabot.yml: restore the uv ecosystem entry for functional-tests/.
  It was dropped with the legacy delete, but the path is unchanged after
  the rename so the new framework still needs Python dependency updates.
- functional.yml: filter the coverage-instrumented cargo build to the
  binaries the new tests actually use, and update the which assertion
  from strata-client to strata.
- FUNCTIONAL_TEST_MIGRATION_STATUS.md: delete the migration tracking
  doc now that the legacy tree is gone.
The btcio tests (added by PR #1451) were added to main after the rename
commit was written. The rebase didn't capture them, leaving orphaned
tracked files under functional-tests-new/tests/btcio/.
@storopoli storopoli dismissed stale reviews from delbonis, prajwolrg, and themself via f9f4bc9 April 13, 2026 18:24
@storopoli storopoli force-pushed the chore/delete-legacy-functional-tests branch from f81f15c to f9f4bc9 Compare April 13, 2026 18:24
@storopoli
Copy link
Copy Markdown
Member

@voidash heads-up I've rebased this branch and will force merge when CI is all green. 🤞🏻

@storopoli
Copy link
Copy Markdown
Member

Ok all green merging this!

@storopoli storopoli disabled auto-merge April 13, 2026 19:03
@storopoli storopoli merged commit 69c9872 into main Apr 13, 2026
31 checks passed
@storopoli storopoli deleted the chore/delete-legacy-functional-tests branch April 13, 2026 19:03
@storopoli
Copy link
Copy Markdown
Member

NOTE to myself. After this PR is merged we need to update the merge queue requirements because of the renaming CleanShot 2026-04-08 at 12 24 52@2x

NOTE to myself. After this PR is merged we need to update the merge queue requirements because of the renaming CleanShot 2026-04-08 at 12 24 52@2x

This is done. updated the merge queue requirements. Cc @barakshani

@purusang
Copy link
Copy Markdown
Contributor

purusang commented Apr 15, 2026

@storopoli this is good that it got merged and I like fast development cycles but there was an unresolved concern of mine. I saw you left some reminders for yourself, I guess, you can incorporate it there.

@storopoli
Copy link
Copy Markdown
Member

You mean the .dockerignore? I thought that was solved. Ok opened up #1651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants