Incomplete testing in Windowing System/Add a method to resize the window

Programming JavaScript
1

in the Task 4: “Add a method to resize the window” the following condition is not being tested:

“The maximum height and width depend on the current position of the window, the edges of the window cannot move past the edges of the screen.”

if the student only creates a resize method that fulfill this condition: “The minimum allowed height or width is 1. Requested heights or widths less than 1 will be clipped to 1.” the task 4 will be complete but will give an error in task 5 when after moving the window then call the resize method.

2

Hey hey, Noticed the same behavior, but this might be more of a QA task on the instructions for the exercise?

If you pull down the exercise using the CLI and check windowing-system.spec.js, you’ll see that the test has the .resize() method as a dependency. So it makes sense that you’d run the test after step 4, but that makes it confusing because you expect to see the result of the unit test immediately after completing the task.

Maybe to help with clarity, the Excercism team could move the instruction into task 5 instead of 4? I’m actually not sure how to make this one more clear, but agree it can be improved somehow – the test works for me though!

3

The exercise is going to be revised to clear all of this up. I don’t know when yet, but it’s on our radar and list :slight_smile:

4

Awesome! I’d really like to contribute to Exercism, so if this a PR that I can take please let me know :slight_smile:

5

I have written a test to try and implement this check into windowing-system.spec.js

Here:

  test('does not resize programWindow above the maximum allowed window size', () => {
    const programWindow = new ProgramWindow();
    const newSize = new Size(850, 1000);
    programWindow.resize(newSize);

    expect(programWindow.size.width).toBe(800);
    expect(programWindow.size.height).toBe(600);
  });

@Endzelisp @alexownejazayeri @SleeplessByte @iHiD

If Endzelisp or alex wants to open a PR for this they should, but if you want me to I can do that as well!

6

You know what, I think there is a test case for this is actually covered. I misunderstood what was required here.

Here is the old test, which I believe is checking the same thing as the other test I had provided:

  test('resize respects limits due to position and screen size', () => {
    const programWindow = new ProgramWindow();
    const newPosition = new Position(710, 525);
    programWindow.move(newPosition);
    const newSize = new Size(1000, 1000);
    programWindow.resize(newSize);

    expect(programWindow.size.width).toBe(90);
    expect(programWindow.size.height).toBe(75);
  });

Instead of adding a new test, couldnt the test just be moved into the describe of the resize() method for 4. Add a method to resize the window, but does it have to include the move() method in the test? That way the tests match the order in which they are asked for in the description?

Honestly the instructions for this exercise are really long, but they actually make sense to me. IMO a lot of Javascript stuff is falling behind but this particular one seems okay to me.

7

Can you elaborate <3 ? Perhaps in a new forum topic!

If TDD then works better, yes please. We’ll accept a PR!

sidenote: once I’m no longer in constant pain, this exercise is still going to be revised, but until then these changes are probably for the better.

8

Hey again, sorry for the late follow up but I went away to get medical treatment and forgot what this is all about.

I will return in a few weeks to continue learning, always :slight_smile:

Didn’t want to leave you on read. Peace

2 Likes