V2 by RoySegall · Pull Request #3 · testimio/JS-interview

RoySegall · 2025-01-21T14:36:29Z

A couple of notes about this one:

I removed the part of the router we never got there
I added a simple file because it seems mid level developers got messed with this
I switched to vitest instead of mocha, or any test runner we had there

Signed-off-by: Roy Segall <roy@segall.io>

omril1 · 2025-01-22T08:25:50Z

README.md

 * Do time yourself (the benchmark is approximately 30 minutes)
 * You may use every resource you would naturally use as a developer, except for AI-based tools such as Co-Pilot, ChatGPT, Bard etc.
-* Please send the completed exercise zipped (.zip, or if the mail blocks just change the extension to something else) to Dror Elovits (d.elovits@tricentis.com) within up to 48h
+* Please send the completed exercise zipped (.zip, or if the mail blocks just change the extension to something else) to Dror Elovits (y.raz@tricentis.com) within up to 48h


Suggested change

* Please send the completed exercise zipped (.zip, or if the mail blocks just change the extension to something else) to Dror Elovits (y.raz@tricentis.com) within up to 48h

* Please send the completed exercise zipped (.zip, or if the mail blocks just change the extension to something else) to Yuval Raz (y.raz@tricentis.com) within up to 48h

Shahar-Chen · 2025-01-22T08:38:38Z

emitter/part1.md

-    var f = new EventEmitter();
-    f.on("Hello", function(data){
+```js
+    var eventEitter = new EventEmitter();


haters with typos?

promise/promise.ts

Shahar-Chen · 2025-01-22T08:41:26Z

Why did we remove the router exercise? you think its not needed?

RoySegall · 2025-01-22T08:44:49Z

Why did we remove the router exercise? you think its not needed?

We never asked it 🤷‍♂️🫤

Signed-off-by: Roy Segall <roy@segall.io>

promise/package.json

Signed-off-by: Roy Segall <roy@segall.io>

emitter/package.json

liorama · 2025-01-22T15:19:16Z

@RoySegall The router is not a bad question for some scenarios. The solution to the problem of not giving it as a question might be, giving it as a question. We might just need to develop it a little. Please don't delete it.

Signed-off-by: Roy Segall <roy@segall.io>

RoySegall · 2025-01-23T17:54:03Z

We might just need to develop it a little. Please don't delete it.

@stefandokic-tricentis refactored it but since he does not have a write permission i copied it. Need to verify it.

README.md

atlowChemi · 2025-01-25T19:19:45Z

emitter/part2.md

-Name your file `emitter.js` and export your class/constructor as `EventEmitter`. After you `npm i` you can execute `npm run test2`.

-**We __encourage__ you to run the tests**.
+Name your file `Emitter.ts` and export your class/constructor as `EventEmitter`. After you `npm i` you can execute `npm run test:emitter:part1`.


Suggested change

Name your file `Emitter.ts` and export your class/constructor as `EventEmitter`. After you `npm i` you can execute `npm run test:emitter:part1`.

Name your file `Emitter.ts` and export your class/constructor as `EventEmitter`. After you `npm i` you can execute `npm run test:emitter:part2`.

Also, what if someone wants to run with bun?

This is so runtime assuming

atlowChemi · 2025-01-25T19:21:10Z

emitter/test/part1.test.ts

+        emitter.on("Hello", done);
+        emitter.trigger("Hello");
+
+        expect(done).toHaveBeenCalled();


This should really also test it has only been called once.

And check it did not happen before .trigger

atlowChemi · 2025-01-25T19:22:36Z

emitter/test/part1.test.ts

+        const handler = vi.fn((data) => {
+            expect(data).toBe("World");
+        });
+
+        emitter.on("Hello", handler);
+        emitter.trigger("Hello", "World");
+
+        expect(handler).toHaveBeenCalled();


Suggested change

const handler = vi.fn((data) => {

expect(data).toBe("World");

});

emitter.on("Hello", handler);

emitter.trigger("Hello", "World");

expect(handler).toHaveBeenCalled();

const handler = vi.fn();

emitter.on("Hello", handler);

emitter.trigger("Hello", "World");

expect(handler).toHaveBeenCalledWith("world");

atlowChemi · 2025-01-25T19:23:25Z

emitter/test/part1.test.ts

+    });
+
+    it("can trigger an event with no handlers", () => {
+        expect(() => emitter.trigger("Hello")).not.toThrow();


Probably best to also check there were no unhandled rejections, you never know what code people write

atlowChemi · 2025-01-25T19:24:28Z

emitter/test/part1.test.ts

+
+    it("supports multiple event triggers same function", () => {
+        const trackedFunction = vi.fn();
+        const trackedFunctionNotToBeInvoked = vi.fn();


You never pass this function anywhere, how would it possibly be invoked?

emitter/tsconfig.json

atlowChemi · 2025-01-25T19:33:56Z

emitter/test/part2.test.ts

+
+    beforeEach(() => {
+        emitter = new EventEmitter();
+        tracker = vi.fn();


You never actually use this?

atlowChemi · 2025-01-25T19:34:45Z

emitter/test/part2.test.ts

+        expect(trackedFunction).toHaveBeenCalledTimes(1);
+    });
+
+    it("can remove and call off twice", () => {


You already tested you can remove, this just checks calling twice won't cause any issues

Suggested change

it("can remove and call off twice", () => {

it("can call off twice", () => {

atlowChemi · 2025-01-25T19:35:20Z

emitter/test/part2.test.ts

+        expect(trackedFunction).toHaveBeenCalledTimes(1);
+    });
+
+    it("removes the relevant event", () => {


Suggested change

it("removes the relevant event", () => {

it("removes only the relevant event", () => {

promise/promiseTimeout.ts

promise/package.json

Signed-off-by: Roy Segall <roy@segall.io>

Shahar-Chen · 2025-01-28T08:46:42Z

Although Chemi has some good comments, I think it looks good enough for the purpose of the code.
Light it up

atlowChemi · 2025-01-30T17:39:22Z

Although Chemi has some good comments, I think it looks good enough for the purpose of the code. Light it up ![]

מתפשרים על שאלות ראיון === מתפשרים על מועמדים, רק אומר

RoySegall added 7 commits January 15, 2025 09:53

Start to refactor the interview repo

7e4ef2e

More stuff

a95b9ac

Signed-off-by: Roy Segall <roy@segall.io>

Adding more

ee6d2b9

Signed-off-by: Roy Segall <roy@segall.io>

more stuff

4db2bad

Signed-off-by: Roy Segall <roy@segall.io>

foo

9fc427d

Signed-off-by: Roy Segall <roy@segall.io>

foo

5c365e0

Signed-off-by: Roy Segall <roy@segall.io>

foo

370372d

Signed-off-by: Roy Segall <roy@segall.io>

RoySegall marked this pull request as draft January 21, 2025 14:36

RoySegall requested review from liorama and removed request for liorama January 21, 2025 14:39

RoySegall added 2 commits January 22, 2025 09:38

more refactring

e3e4089

Signed-off-by: Roy Segall <roy@segall.io>

oops

9acabfe

Signed-off-by: Roy Segall <roy@segall.io>

RoySegall requested a review from liorama January 22, 2025 07:46

RoySegall marked this pull request as ready for review January 22, 2025 07:46

more stuff

0eaa927

Signed-off-by: Roy Segall <roy@segall.io>

omril1 reviewed Jan 22, 2025

View reviewed changes

Shahar-Chen reviewed Jan 22, 2025

View reviewed changes

promise/promise.ts Show resolved Hide resolved

foo

c8407c8

Signed-off-by: Roy Segall <roy@segall.io>

omril1 reviewed Jan 22, 2025

View reviewed changes

promise/package.json Show resolved Hide resolved

bla bla bla...

10dab56

Signed-off-by: Roy Segall <roy@segall.io>

omril1 reviewed Jan 22, 2025

View reviewed changes

emitter/package.json Show resolved Hide resolved

Adding the route

3d5e897

Signed-off-by: Roy Segall <roy@segall.io>