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

Switch to using eos-system-contracts in libtester tests #1178

Merged
merged 2 commits into from
Feb 19, 2025
Merged

Switch to using eos-system-contracts in libtester tests #1178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1303e3e
Revert "switch to using reference-contracts in libtester tests"
greg7mdp Feb 18, 2025
874bbc6
Use `main` branch.
greg7mdp Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .cicd/defaults.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"target":"4.1",
"prerelease":true
},
"referencecontracts":{
"eossystemcontracts":{
"ref":"main"
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to not set it back to a release too?

Copy link
Contributor Author

@greg7mdp greg7mdp Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I feel that main is better, but maybe I'm not understanding the process well (my first time doing this)

For example I am working on some changes in eos-system-contracts , and for the tests to pass I'll update defaults.json to use my branch which has the changes. After my branch is merged to main, I think we'll have to switch defaults.json back to main for the tests to continue passsing (assuming I have merged my eos-system-contracts branch by then).

Copy link
Member

Choose a reason for hiding this comment

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

I think the easiest way to imagine a problem with main is if spring's release/1.2 is using main, and spring's main is using main, and then system contract's main changes in such a way that it requires spring's main. Now it's impossible for any PRs to release/1.2 to pass since they'll still fetch contract's main that won't work on release/1.2.

So it's probably preferable to stick with something stable unless needing a branch for some reason, and at the very least when we prepare to branch a release branch we should try to pin it down.

Keep in mind too that reference contracts has no release branches, so that's a big reason it was left to languish as main even though that's not ideal.

All that said, eos-system-contracts is using main for spring and main for cdt so 🤷‍♀️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

system contract's main changes in such a way that it requires spring's main

That would require a hard fork, right?
But yes I see now that the previous release branches of spring using main would have to be updated for the tests to pass. I guess the developer making a change to a release branch would see a ci/cd test failure, and would have to pin the branch for eos-system-contracts for the tests to pass. Maybe that is the right time to pin the branch ref?

Copy link
Member

Choose a reason for hiding this comment

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

No it does not require a hard fork. It could simply be a new interface that libtester provides that contracts tests begin to use.

}
}
32 changes: 16 additions & 16 deletions .github/workflows/build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ on:
- default
- true
- false
override-reference-contracts:
description: 'Override reference contracts ref'
override-eos-system-contracts:
description: 'Override eos-system-contracts ref'
type: string

permissions:
Expand Down Expand Up @@ -57,26 +57,26 @@ jobs:
outputs:
cdt-target: ${{steps.versions.outputs.cdt-target}}
cdt-prerelease: ${{steps.versions.outputs.cdt-prerelease}}
reference-contracts-ref: ${{steps.versions.outputs.reference-contracts-ref}}
eos-system-contracts-ref: ${{steps.versions.outputs.eos-system-contracts-ref}}
steps:
- name: Setup cdt and reference-contracts versions
- name: Setup cdt and eos-system-contracts versions
id: versions
env:
GH_TOKEN: ${{secrets.GITHUB_TOKEN}}
run: |
DEFAULTS_JSON=$(curl -sSfL $(gh api https://api.github.com/repos/${{github.repository}}/contents/.cicd/defaults.json?ref=${{github.sha}} --jq .download_url))
echo cdt-target=$(echo "$DEFAULTS_JSON" | jq -r '.cdt.target') >> $GITHUB_OUTPUT
echo cdt-prerelease=$(echo "$DEFAULTS_JSON" | jq -r '.cdt.prerelease') >> $GITHUB_OUTPUT
echo reference-contracts-ref=$(echo "$DEFAULTS_JSON" | jq -r '.referencecontracts.ref') >> $GITHUB_OUTPUT
echo eos-system-contracts-ref=$(echo "$DEFAULTS_JSON" | jq -r '.eossystemcontracts.ref') >> $GITHUB_OUTPUT

if [[ "${{inputs.override-cdt}}" != "" ]]; then
echo cdt-target=${{inputs.override-cdt}} >> $GITHUB_OUTPUT
fi
if [[ "${{inputs.override-cdt-prerelease}}" == +(true|false) ]]; then
echo cdt-prerelease=${{inputs.override-cdt-prerelease}} >> $GITHUB_OUTPUT
fi
if [[ "${{inputs.override-reference-contracts}}" != "" ]]; then
echo reference-contracts-ref=${{inputs.override-reference-contracts}} >> $GITHUB_OUTPUT
if [[ "${{inputs.override-eos-system-contracts}}" != "" ]]; then
echo eos-system-contracts-ref=${{inputs.override-eos-system-contracts}} >> $GITHUB_OUTPUT
fi

package:
Expand Down Expand Up @@ -328,21 +328,21 @@ jobs:
rm ./*.deb

# Reference Contracts
- name: checkout reference-contracts
- name: checkout eos-system-contracts
uses: actions/checkout@v4
with:
repository: AntelopeIO/reference-contracts
path: reference-contracts
ref: '${{needs.v.outputs.reference-contracts-ref}}'
repository: eosnetworkfoundation/eos-system-contracts
path: eos-system-contracts
ref: '${{needs.v.outputs.eos-system-contracts-ref}}'
- if: ${{ matrix.test == 'deb-install' }}
name: Install reference-contracts deps
name: Install eos-system-contracts deps
run: |
apt-get -y install cmake build-essential
- name: Build & Test reference-contracts
- name: Build & Test eos-system-contracts
run: |
cmake -S reference-contracts -B reference-contracts/build -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=On -DSYSTEM_ENABLE_SPRING_VERSION_CHECK=Off -DSYSTEM_ENABLE_CDT_VERSION_CHECK=Off
cmake --build reference-contracts/build -- -j $(nproc)
cd reference-contracts/build/tests
cmake -S eos-system-contracts -B eos-system-contracts/build -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=On -DSYSTEM_ENABLE_SPRING_VERSION_CHECK=Off -DSYSTEM_ENABLE_CDT_VERSION_CHECK=Off
cmake --build eos-system-contracts/build -- -j $(nproc)
cd eos-system-contracts/build/tests
ctest --output-on-failure -j $(nproc)

all-passing:
Expand Down