Conversation
0e4db21 to
2f8bbc6
Compare
2f8bbc6 to
f1dc368
Compare
JulienVig
left a comment
There was a problem hiding this comment.
The objective of issue #1196 is to test browser-browser collaboration while the current implementation implements browser-Node.js collaboration. For federated learning this doesn't make a big difference since participants don't directly interact with each other but for decentralized learning the current approach doesn't really address the issue. I'm curious if there was a specific reason that pushed you towards relying on Node.js?
Also, I find the current implementation a bit overkill for what it does, do we actually need such a harness? I've pushed a temporary commit with an alternative implementation that doesn't rely on a harness or a separate cypress config (feel free to remove my commit). Were there cases that couldn't not be addressed in a simpler manner?
Let's discuss these points in tomorrow's meeting!
| cy.contains("button", "Start training").click(); | ||
|
|
||
| cy.contains("h6", "number of participants") | ||
| .next({ timeout: 240_000 }) | ||
| .should("have.text", "2"); | ||
| cy.contains("h6", "epochs") | ||
| .next({ timeout: 240_000 }) | ||
| .should("have.text", "10 / 10"); | ||
| cy.contains("h6", "Collaborative model sharing") | ||
| .next() | ||
| .should("have.text", "5"); | ||
| cy.contains("Training successfully completed"); |
There was a problem hiding this comment.
Can you check that not error toasters are being shown?
| describe("federated webapp training", () => { | ||
| afterEach(() => cy.task("stopFederatedParticipant")); | ||
|
|
||
| it("trains Titanic with a Node participant through a real server", () => { |
There was a problem hiding this comment.
Can you also test lus_covid?
| "../../tsconfig.base.lib.json" | ||
| ], | ||
| "include": ["./e2e/**/*", "./support/**/*"], | ||
| "include": ["./collaborative/**/*", "./e2e/**/*", "./support/**/*"], |
There was a problem hiding this comment.
why not make the collaborative folder a subfolder of e2e since it is in fact an end-to-end test
There was a problem hiding this comment.
We can put it under e2e, we just have to exclude their discovery under regular testing.
By default cypress discovers the tests in the path provided by specPattern in the config, which is "cypress/e2e/**/*.cy.{js,jsx,ts,tsx}" by default.
Since the collaborative tests need a server the setup is a bit different than the other tests
45fccc4 to
0b5c954
Compare
What & why
Part of #1196. This adds webapp end-to-end coverage for real federated training using the default Titanic task. One browser participant trains through the full UI alongside a Node participant against a real Disco server, covering browser networking, model exchange, aggregation progress, and successful completion. It also avoids passing Node-only WebSocket options to the native browser constructor.
Technical plan
A dedicated Cypress configuration starts and owns a minimal test harness containing the real server and background Node participant. The harness runs as a Node 22 child process so native TensorFlow does not load inside Cypress's bundled Node 24 config runtime. Separate start and await tasks allow the browser and Node participant to train concurrently while propagating background failures. Teardown is bounded, and a dedicated CI job runs only this collaborative spec while leaving the ordinary Cypress suite unchanged.
Deviations from plan
The server and Node participant were moved from the Cypress config process into a child-process harness after CI exposed that
@tensorflow/tfjs-node@4.22.0is incompatible with Cypress's bundled Node 24 runtime. The test scenario and assertions are unchanged.Todo
Cover decentralized browser training separately after validating browser-to-peer WebRTC interoperability. Extend collaborative coverage to further default tasks separately if needed.