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.

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

forest Test #59

Merged
merged 9 commits into from
Jul 12, 2024
Merged

forest Test #59

merged 9 commits into from
Jul 12, 2024

Conversation

snadrus
Copy link
Contributor

@snadrus snadrus commented Jun 15, 2024

Create a CircleCI test to alert whenever Curio begins using a chain API that is not one we have informed the Forest team about.

In creating this, 3 new API usages were discovered and shared via email.
SPTool & Boost are not included here.
"enforcement" applies to the use of the API from /deps/.

Direct uses (like SPTool & Boost do) are not tracked. Please use /deps/deps.go's chain API. It offers the entire lotus API, but errors in testing.

Got a testing error? Just add the API to /api/api_chain.go's ChainSubsetForForest andn share it with the Forest team.

@snadrus snadrus requested review from magik6k and LexLuthr June 15, 2024 00:17
Copy link
Contributor

@LexLuthr LexLuthr left a comment

Choose a reason for hiding this comment

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

I think we are using v1API in couple of more places in web. One would be for chain sync status and there is one more in simpleinfo under hapi.

@LexLuthr
Copy link
Contributor

I am not sure if this is the final or first draft as the approach here ties us to a build. We probably don't want that. User's shouldn't need to build specific to nodes. Tests should also be independent in similar manner.

@snadrus
Copy link
Contributor Author

snadrus commented Jun 18, 2024

I am not sure if this is the final or first draft as the approach here ties us to a build. We probably don't want that. User's shouldn't need to build specific to nodes. Tests should also be independent in similar manner.

Actually, we can target the same full API for either Lotus or Forest as long as we know that the subset we use is Forest-safe. This code adds a unit test to ensure such a thing is true.

Copy link
Collaborator

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Might be simpler to always directly use the Curio subset interface, the aliases shouldn't really be needed.

Also would be neat to have some test which e.g. tries to comment out the methods in the interface one at a time, and if it manages to compile curio with a method commented out it would complain loudly that a method appears to be unused

api/proxy_gen.go Outdated Show resolved Hide resolved
api/api_chain_all.go Outdated Show resolved Hide resolved
api/api_chain.go Outdated Show resolved Hide resolved
api/api_chain.go Outdated Show resolved Hide resolved
api/api_chain.go Outdated Show resolved Hide resolved
deps/apiinfo.go Show resolved Hide resolved
@snadrus snadrus requested review from magik6k and LexLuthr July 10, 2024 22:24
@snadrus snadrus merged commit a1b7625 into main Jul 12, 2024
1 check was pending
@snadrus snadrus deleted the feat/forest3 branch July 12, 2024 16:27
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.

3 participants