Improving tests fragility

[viferga]

After the amount of PRs, I have closed some because it's going to be hard to test, or they did not have tests directly.
I realized that we should improve how to test metacall/{deploy, faas, protocol} repositories. Right now it's very fragile.

I was thinking to implement a trigger or something from this repo, that replaces the fixed version of metacall/protocol into metacall/faas and metacall/deploy, and it tests with lasts changes done. This should be executed on each commit probably.

Also there should be a way to easily tests the PRs of metacall/deploy with full integration tests but finding a way to maintain security, because those tests require to use secrets related to the MetaCall FaaS Dashboard.

We should also incrementally migrate all the tests we got using Bash for something more standard and Typescript.

We should elaborate this more, but I think this is the direction to go.

[miantalha45]

Yes @viferga you are right. Currently the test mechanism is not too good and reliable. And also the direction you mentioned is looking good to me. I would make some search about it and let you know.

[aditya-8787]

Sir @viferga,I have been going through the testing issue for metacall/protocol, metacall/faas, and metacall/deploy and wanted to check if I am thinking about this correctly before I start.
My idea is basically three things:
First, whenever something is pushed to metacall/protocol, automatically trigger CI in both faas and deploy using repository_dispatch, and swap out the pinned protocol version with the latest code using npm link so we are always testing against real changes, not a fixed snapshot.
Second, for the integration tests in deploy that need secrets like METACALL_API_KEY , I was thinking of using pull_request_target with a safe-to-test label that only maintainers can apply, plus a protected GitHub Environment as an extra layer. That way secrets never leak to fork PRs unless someone explicitly approves it.
third instead of migrating test by test, I was thinking of setting up a parallel test suite keep the existing Bash tests running as they are, and build the TypeScript/Vitest setup alongside them from scratch. New tests going forward are always written in TypeScript, and the Bash ones only get removed once the entire suite has equivalent coverage verified. This way there's a clean cut at some point rather than a long dragged-out migration that never really finishes.
Does this sound like the right direction to you? Let me know if I am off on anything or should approach it differently.
Thanks

[Amanlabh]

I added a downstream CI workflow that packs the PR build of @metacall/protocol and runs deploy unit tests + faas tests against it (no secrets required). This helps catch cross repo breakages early and improves test reliability. PR: #54

[viferga]

Instead of asking AI how to improve this, you should discuss it with me. I have closed all PRs because they did not make any sense. If you want to know how to implement this you should ask me, no the AI.

[aditya-8787]

@viferga, I want to clarify something before this gets closed. The three points I proposed — the repository_dispatch trigger, the
safe-to-test label for secrets, and the parallel Vitest migration —
were all things I discussed with you directly before writing any code,
and you confirmed the direction was correct. I did not just ask AI to
generate a solution and push it. I actually ran everything locally, all
11 tests pass on my machine, and I made sure the build was clean before
raising the PR.

I agree the PR could have been explained better and I take
responsibility for that. But the work itself came from our conversation
and I understood what I was building.

That said, I do not want to assume anything. If the implementation is
not what you had in mind, I would rather ask you directly than guess.
Specifically I want to confirm — for the cross repo trigger, you
mentioned using the package as it is on current master on GitHub, should
I be using npm install metacall/protocol#master in the faas and deploy
workflows, or did you have a different approach in mind? And is there a
specific part of this you want me to start with first?

I am happy to redo or restructure anything. I just want to make sure I
am building what you actually want. Thanks for your time.

[viferga]

@aditya-8787

I have reopened it. Let me review it in more detail...

[aditya-8787]

@viferga sir I guess there is some mistake you did not open my pr yet.

[miantalha45]

@aditya-8787 propose a good idea for testing but i think it have some flaws.
using npm link in a CI environment creates unpredictable builds, npm link uses global symlinks and often resolves dependencies incorrectly across workspaces.
Even with a "safe-to-test" label, a bad actor can write malicious test code that reads the injected secret and sends it to an external server.
and npm install metacall/protocol#master will also fail to get the build/dist folder because that is not commited to repo. (this will cause the same error as we face earlier by switching to specific commit @viferga ).

[aditya-8787]

@miantalha45 Before I proceed further, I wanted to share my plan
for the next steps and get your confirmation that I am going in the
right direction.

Here is what I am planning next:

protocol repo — Add a trigger-downstream job to the existing CI
that pings metacall/faas and metacall/deploy via repository_dispatch
after every master push, so they can test against latest protocol
instead of a pinned version.

faas repo — Add a small workflow that listens for the
protocol-updated event, replaces the pinned protocol version with
metacall/protocol#master and runs existing tests.

deploy repo — Same as faas, plus split into two jobs: unit tests
run on all PRs normally, integration tests only run when a maintainer
applies a safe-to-test label so METACALL_API_KEY is never exposed to
fork PRs.

Does this align with what you have in mind? Let me know if I should
approach anything differently before I start. Thanks

[viferga]

This is solved apparently, we must test a PR now, but still I want to revert all the changes and leave them in a branch called unstable, then cherry-pick them one by one to verify it works.