WIP: forgejo - gotosocial federation testing #664
earl-warren
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "famfo/end-to-end:gts-testing"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What is being tested:
Notes) from gts -> forgejoCI output to testcases
Dependencies
This largely depends on https://codeberg.org/forgejo/forgejo/pulls/4767. I again, have some local fixes ontop of that, which are at https://codeberg.org/meissa/forgejo/pulls/19.
TODOs:
run.shscript, I want to get rid of it once the gts PR has been merged@ -0,0 +27,4 @@-v "$tmpdir:/mount" \-p "8080:8080" \--entrypoint "/mount/gotosocial" \docker.io/superseriousbusiness/gotosocial:latest \It would be fine here to use an image of your own that includes your patch.
@ -0,0 +41,4 @@--password "verysecurepassword"echo "waiting 5"sleep 5speed can vary widely, instead of sleeping loop over a command that checks it is done (retry(1) is available)
@ -0,0 +4,4 @@#patchelf ../forgejo/forgejo --set-interpreter /lib64/ld-linux-x86-64.so.2podman run \-it --privileged \-v ../forgejo/gitea:/srv/forgejo-binaries/forgejo-11.0 \For testing purposes you can build from a designated repository. All you need to do is something like:
#662/files
and
end-to-end/lib/build.shwill compile into forgejo v12.0 which you can then use.I will refrain from commenting on details before it is actually working. But it looks like it is going in the right direction, from a distance.
You can move the Dockerfile into a function in
lib/lib.shthat installs podman (it will be used in other contexts I'm sure) and call that function fromfederation/federation.sh. This way all you've done can assume podman is already installed and that should simplify the setup a bit.The log patches are good to merge right away, of course.
@ -74,0 +76,4 @@$SUDO apt-get update -qq$SUDO apt-get install -y -qq podmanfi}It would be fine to always install podman in
function dependency() {at$SUDO apt-get install -y -qq make curl daemon git-lfs jq sqlite3 skopeoWIP: forgejo - gotosocial federation testingto forgejo - gotosocial federation testingCan you please update the description with:
I see you are using a gts release, does that mean the PR you were waiting for was merged already?
For the sake of clarity you should
The commit was merged, the gts tag I'm using is built from the latest main commit.
How are commits squashed together on merge? Is it just all a single commits or should I subdivide them a bit more?
forgejo - gotosocial federation testingto WIP: forgejo - gotosocial federation testing649d4b5b78e24c44800f@famfo wrote in #664 (comment):
They will be merged if they are nicely organized or squashed if they are not. It is up to you which way you prefer. What you've done now is good.
Could you point to the relevant lines in the full log? this line and above. This is to make it easier for me (or another reviewer) to know where to look to check this is not a false positive and that the assertions are meaningful. 🙏
@famfo wrote in #664 (comment):
Congratulations on that 🎉
e24c44800fc71e61e2b8Changed the branch to the one used in https://codeberg.org/forgejo/forgejo/pulls/4767, I could also use the ref from
forgejo/forgejodirectly if that is more desirable :)@famfo it is fine as it is.
@earl-warren wrote in #664 (comment):
I see you have added a link to the description. However this is not quite what I meant. Could you please help by matching the most relevant output lines (out of hundreds of output lines) with the corresponding lines in the scenario to help me with the review? It will otherwise make it more difficult for me. It is easier for you because you know what to look at and can explain: "this works (link to script) because it shows this output (link to output)".
Also note that there are no notifications when you edit the description or when you add an emoticon. Reason why I did not reply right away. Please post a comment to draw my attention 🙏
I have tried to link the CI output and test cases in a readable manner :)
@ -0,0 +29,4 @@server start)"function wait_forgejo_migrations() {you mean
wait_gts_readyhere or am I misreading this?This is a very fine and simple end-to-end test ❤️ With the guidance provided, the review is very simple. I will do the same manually on my local machine today. Exciting times.
If you rebase all checks shoudl pass. The upgrade failure is a false negative.
Could you split out the Dockerfile + podman installation in a separate PR? It can be merged right away and will reduce the scope of this PR.
c71e61e2b8a4a0c5f49aSplit off docker and logger updates, not sure if the logger update is maybe a bit too ambitious for older versions :)
Edit: logger update tests pass, all good :D
GoToSocial currently seems to have a broken
main. I'd potentially hold this PR until gts has a new tagged release as well.@ -81,3 +89,2 @@for scenario in star; dorun federation_verify_scenario $scenarioif [[ -z "${setup[$version[1]]}" ]]; thenPlease remove the if: all versions are different now so it won't make a difference. If there is a need to optimize later on, it can be done when that problem shows.
a4a0c5f49a1672db8d30The upgrade test failure is related
https://code.forgejo.org/forgejo/end-to-end/actions/runs/3179/jobs/6#jobstep-3-1137
it needs fixing in the migration of the corresponding PR.
You did not implement a test for unfollowing a user. Is it because it is not implemented yet in the Forgejo pull request or because you did not get to it yet?
gts -> forgejo unfollowing should work, the other way around is still missing an API endpoint iirc. I'll ad that to the tests
1672db8d30d982518ac6Well, this test is good for symmetry but (unless I'm mistaken) it does not imply any action Forgejo side, it is merely GtS side updates. Is that correct?
Side question: have you tried following Forgejo from Mastodon or just from GtS? If you did, do you remember why it did not work?
So far, I have only tested with gotosocial because that has been the easiest to test with and hack on. I'd like to try Mastodon or other software eg. Akkoma as well in the future.
There should be action on the forgejo side https://codeberg.org/forgejo/forgejo/pulls/4767/files#diff-4dd164c79af275bd2ae3e3a011cf1758fb597d06
I just didn't look at it in the new testcase 😅
Both the
followersand thefollowingforgejo API endpoints do not seem to list federated follows so they can't be used for verification yetIt is fine that those are not implemented yet. There however is a need to assert the side effect is happening. 🤔 @jerger do you have a suggestion?
What about this
Get all status from GtS and find only A1 and A3. This is very high level but would show that unfollow worked because of the missing note that reflects something that happened while the user was not followed.
Note: it is possible that what I propose won't work because that's not how activitypub works....
Only A1 and A3 should be explicitly found in the inbox of U. I can try to test with this a bit later today :)
That's good news. It will also verify that re-following the same user works, which confirms unfollowing does not leave leftovers behind.
Hm, a thought I have on the potential test scenario of following unfollowing and following again: if backfill of posts would be implemented this isn't a reliable test case anymore
I'll try to think about this a bit more
This is a problem for the future though and I would not mind something that works now. In the future tests will also be able to rely on the list of followers / followings which are not available at the moment so it won't be an issue.
It would really be great to get in a state where this feature can be documented to be available for everyone to use and it seems there is very little required to make that happen in the following days.
Restarted the federation test only so that it runs out of the latest commit from the associated pull request.
Will you have time to add the unfollowing test? It would help a lot with the final review of the pull request, to confirm it does what it should 🙏
Been a bit busy the last few days, I'll try to get something together this evening hopefully :)
That would be great ❤️
d982518ac67ecb188d47Added the test case and updated the CI output match
Excellent.
ping @jerger the upgrade failure at https://codeberg.org/forgejo/forgejo/pulls/4767#issuecomment-5416143.
Could you please give it a spin to figure out if it still works given the changes in the past three weeks?
7ecb188d4725d7ae405fRebased and had to remove the
--quietfrom grep, not sure why curl did not like it.Anyways, the tests still pass against the latest changes on the PR (the CI should also run again now :>)
Hm the CI did fail, I'll do some more debugging on this later
Later turned into 5 days, something is either up with the CI or inconsistent at best. The test consistently passes locally but forgejo and gotosocial fail to properly communicate inside the CI
25d7ae405f40a26b459640a26b4596793eef2845Moved the build from source to 13.0 and set the minimum for the scenario to run to 13.0 as well
793eef2845a8feea96e3a8feea96e348b12a0ff848b12a0ff81c3ee9eb1bRemoved the
--quietfrom grep and curl can write it's output again. I'm slightly confused.For better log readability I can maybe just redirect the grep output to /dev/null again.
Ah, the output is not shown in the log anyways. Good enough then :D
1c3ee9eb1b643004fdc0There has been a new gotosocial release which includes my changes for testing, I'll rebase onto it in a few days when I have time to hack on forgejo again.
643004fdc057649677d4Finally updated gts to
latest, aka 0.20 which has been released now :)@famfo I canceled & re-ran the CI (reboot of the instance)
The UI is no longer wonkey :D
Hm not sure why the tests failed, I'll look at it in the next few days
Oh no, what the hell
I will close this as all changes picked from here have been merged in some form.
Pull request closed