Use cases are completed in devlp-env but not when running-tests

I’ve completed this challenge & currently waiting on publishing. When I run it in my development environment all of the things are working according to the user-scenarios but the problem is only 75% of those scenarios working (23/29 test cases) as expected with the FCC test integration.
All of the logics were implemented using standard ES6 JS & not included any Promises & Async funcs. I’m currently struggling to figure out why some of the simplest test cases wont pass during test-phase even when they provide the expected output when validating without FCC test-integration.

My code so far

Browser information:

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:125.0) Gecko/20100101 Firefox/125.0

Challenge Information:

Front End Development Libraries Projects - Build a 25 + 5 Clock

The main issue is that tests sometimes don’t finish within the expected time frame. You’ve written it in ES6 standard, but it doesn’t related with that. However, you’ve used an older React class method, which might cause DOM manipulation operations to not respond quickly enough and throwing informative error message about if you use hook or async function. If you explain the operations you’ve performed for clarity, we can debug the performance and errors in your code.

One of the initial problems I encountered is that after reducing the session-length or break-length values, if it’s reduced to 1, it cannot be increased again.

Moreover, even if the values are not changed, for example, after the session time is over and the break time begins, it’s only updated as 1 minute in the countdown, and after it finishes, the session time (whatever value is set) is again updated as one minute. So, the values of the cyclical session-break timer, which should be updated correctly, are not.

Could you please explain the ManageSession function? How is the interval being refreshed, and how does it retrieve the break and session values? How does it work? How do you manage and update the state?

Additionally, you don’t need to use Try-Catch within ManageSession. There are no async functions or fetching operations here. Possible syntax errors can be caught and displayed by the compiler.

Furthermore, despite these issues, there’s another problem where up to 27 tests pass without any changes in the code. The tests set the break time to one minute, so they recognize the new value correctly, but as I mentioned, updating the session and break times doesn’t make the countdown work correctly.

tnx for the response Mr. @0x74h51N , let me answer & explain questions u mentioned one by one …

One of the initial problems I encountered is …

  1.   now that u have mentioned I’ve tried it & this error doesn’t happen in my development environment. But it does happen on CODEPEN & I don’t know the reason why it happens in CODEPEN.

Moreover, even if the values are not changed, for …

  1.   yes this is new. I’ve to look-in to my code again to figure this out. good thing u hv noticed that.

Could you please explain the ManageSession function?

  1.   I’ve built two different functions to interact with user actions

                      • UpdateSession() - changes the relevant value for break & session-length (also updates the state - either break_value or session_length )
                      • ManageSession() - Start / Stop (Pause) / Reset the timer on control btn click (updates the whole state when each action triggers)


4. ManageSession( param );

   based on the parameter value operations distributed among three categories within a if-else_if-else condition block (‘reset’, ‘pause’, ‘start’ consecutively). a countdown was implemented using a JS setInterval() method and assigned it to the app state itself (session_intervalID).

  • on ‘reset’ [ if statement ]
        first the audio element will be paused. after that based on the app-state’s “session_intervalID” value (default is set to ‘0’) if there’s a running interval it will be cleared then the whole state is set to defaults.

  • on ‘pause’ [ else-if statement ]
       this action is only executable during an ongoing countdown session. if there’s a countdown going on it will be cleared. then the current holding value in the <#time-left> element will be assigned to the updating state’s “session_time”. accordingly the “session_state” will set to ‘running’, “session_paused” will set to ‘true’ & finally “session_intervalID” will reset to ‘0’ & updates the app-state.

  • on ‘start’ [ else statement ]
       at the beginning the countdown timer (newIntervalID) is settled using JS setInterval() method. there is nothing much to explain within this countdown interval. the only important logic remain to explain is after comparing string value from <#time-left> element, the value will be converted to integer & reduced by ‘1’ (if minutes == 0 → minutes remains the same, if seconds == 0 → seconds will reassigned to ‘60’).
       then the updated minutes & seconds will insert to a setState() (“session_time” updated). after that based on the ongoing “session_type” next countdown iteration time will checked whether its a ‘break-time’ or a ‘session-countdown’ (if it’s ‘break’ updating time get assigned to the ‘break-length’ else ‘session length’).
       then after, if the current displayed time value is equals to “00:00” a simple if condition will evaluated. within that if condition as the first step the audio #beep will be played & then after passing 1 second of time the whole app-state will be updated including the newly assigned “session_time” (‘break’ or 'session) & without the “session_intervalID”. by ending this if statement the ‘newIntervalID’ will finalized.
       finally, the app-state will be updated to assign the ‘newIntervalID’ to “session_intervalID” and so on…

Additionally, you don’t need to use Try-Catch within …

  1. Ah, I wasn’t aware of that. I included them as a precautionary measure, anticipating that any error would be directed to the console rather than being displayed in the UI. should I remove it? is it unnecessary or no-point of at all?

Furthermore, despite these is…

  1. sorry, I didn’t get a clear idea about what u said here. As I understood without doing any changes my code passed 27/29 test cases when u run it right? but I’ve tried multiple times & the only highest test-case count I’ve passed is 24 (newly including the 3rd audio test - doesn’t completing every time & the static pass count is 23 without this - don’t know why)


    Hope my explanation will help u to understand wt I had done. ThankU for sparing ur valuble time to help beginners like me. :smiley: :v:

UPDATE | for Error no: 01 - its a logic error, my bad :upside_down_face: I fixed it. updating CODEPEN soon

Please, let’s avoid using “Mr.”, it’s quite unnecessary.

The reason why the initial time doesn’t transition from sessionLength to breakLength is due to your use of timeout. Especially, a one-second timeout after the time has elapsed disrupts the interval’s duration. You can try to control it by clearing the timeout, but it would be healthier to avoid using it altogether. This is because, in this class-based React approach, you don’t have tools like useEffect to clean up timeouts based on dependencies, making it a bit more challenging to manage. For this, you need to use tools like componentDidMount(), componentWillUnmount(), etc.

      const newIntervalId = setInterval(() => {
          let minutes = (parseInt(countdown.innerHTML.split(':')[1])!==0) ? countdown.innerHTML.split(':')[0] : 
            ((parseInt(countdown.innerHTML.split(':')[0])!==0) ? (parseInt(countdown.innerHTML.split(':')[0])-1).toString() : countdown.innerHTML.split(':')[0]);
          let seconds = (parseInt(countdown.innerHTML.split(':')[1])===0) ? "60" : countdown.innerHTML.split(':')[1];
          
          minutes =  minutes.length<2 ? '0'+minutes : minutes;
          seconds = (parseInt(seconds)-1).toString().length<2 ? '0'+(parseInt(seconds)-1) : (parseInt(seconds)-1).toString();
          // countdown.innerHTML = minutes+':'+seconds;
          setTimeout(() => this.setState({ ...this.state, session_time: (minutes+':'+seconds)}),10);

          let updated_time;

          if(this.state.session_type==="countdown") {
            updated_time = ((document.getElementById("break-length").innerHTML.length<2) ? 
              '0'+document.getElementById("break-length").innerHTML : document.getElementById("break-length").innerHTML)+":00";
          } else {
            updated_time = new Date(`1970-01-01T00:${document.getElementById("session-length").innerHTML}:00`).toTimeString().split(' ')[0].split(':');
            updated_time = updated_time[1]+":00";
          }
          
          if(countdown.innerHTML==="00:00") {
            document.getElementById("beep").play();

            setTimeout(this.setState(prevState => { return {
              session_break: parseInt(document.getElementById("break-length").innerHTML),
              session_length: parseInt(document.getElementById("session-length").innerHTML),
              session_time: updated_time,
              session_state: "running",
              session_paused: false,
              session_type: (prevState.session_type==="countdown") ? "break" : "countdown"
              // ,session_intervalID: 0
            }}),1000);
          }

          return 0;
        }, 1000);

But first and foremost, consider keeping the time state only as seconds and perform the conversion only when needed, such as when sending it to the screen or when updating input that will change this state. This will make your code both simpler and healthier. In the current version, the state you hold as session_time is not in a pleasant and manageable format; it will be much easier to manage if you keep it as an integer representing seconds, increasing and decreasing it.

Additionally, you should perform DOM updates in a place independent of all other operations. You can directly use or update the state on the DOM element. Inside the interval and other controls, you should only fetch the value from the state and update it, no need to access to DOM elements in here. Also mostly you don’t need to use document.getElementById in react. You can access directly to the DOM elements return part of JSX file and update with satates.

Yes, you don’t need try-catch. Also, the questions I asked were aimed at explaining and analyzing how your program works directly from the code. For example, why are you updating setState almost identically twice when type===‘reset’? Or why are you assigning an ID to the interval? Simply clearing and restarting the same interval when the type of season (break or session) changes would suffice.

Additionally, it’s a great idea to create a separate component for setting the season lengths. However, you don’t need to pass all the IDs and variable names to this component. For example, you can just pass the type prop and append the changing session or break expressions to the IDs. Also class can be same, all stills are same or you can acess with id if you need it.
Sample usage:

        <Timeset
          setTimeFrame={(e) => this.UpdateSession(e)}
          _type="session"
          _val={this.state.session_length}
        />
       <Timeset
          setTimeFrame={(e) => this.UpdateBreak(e)}
          _type="break"
          _val={this.state.break_length}
        />

In conclusion, learning and using React with hooks would be much more appropriate both for performance and self-improvement. The usage of class-based components has significantly decreased after the release of version 16.8 in 2019. It’s true that this is how it’s taught in fCC lessons, but you’re right, you can update your knowledge with online resources if you’d like. Also I’ll try to help you again…

1 Like

The DOM should reflect the internal app state. You never read from the DOM to set the state, the DOM is not where your state lives, it is where it is displayed.

You should have an internal running clock and its state should be formatted and displayed to the DOM.

Think of a physical, real world digital stop watch. The internal mechanism does not depend on its display, it doesn’t read from its display to know what state it is in, the display is just the visual reflection of its internal mechanism.


There is no issue with using class components here. Knowing both is valuable (plenty of legacy code out there) but I would suggest learning the function/hook based approach as well.

2 Likes

Yes you are right @lasjorg. I’ve misexpressed what I meant about the DOM. Let me correct that.

By updating the state, I meant updating states like break_length or session_length using event listener functions of buttons. So update to state in DOM was wrong expression.

Apart from that, I tried to say that in the interval, by updating the state and directly using the updated state in DOM elements, values can be displayed without interfering with the DOM using getElementById.innerHTML to update displayed values.

1 Like

@0x74h51N Noted! :handshake:. Also, I’ve fixed those unnecessary timeouts & restructured the code.

@lasjorg , are u refering to this ?

if(countdown.innerHTML==="00:00") { /* logic */ }

I’ve change this to :

if(this.state.session_time==="00:00") { /* logic */ }

I dont hv a clear idea to set a proper clearInterval() within a class component. wt I'v done here is how I managed to restart or clear-out the Interval using the help of app-state. if this is not the proper way, can u explain how should I implement a proper Interval-timeout within a class function (besides using a functional component to replace existing component)?

now that I’ve made the necessary changes, my locally hosted app passes all 29 tests but my CODEPEN randomly jump between 11-23 success test-cases. any Idea why this happening? in multiple failed test cases indicate that the #time-left has not reached “00:00” even it has.

Well, that is one example. But the code inside your setInterval is doing it as well. You shouldn’t read the countdown DOM element for the state update.

The time can be a single value, like seconds, and using that value you can derive the other values, like minutes, using math.

For the formatting, one option is the toISOString Date method.

new Date(25 * 1000).toISOString().substring(14, 19)
'00:25'
new Date(1500 * 1000).toISOString().substring(14, 19)
'25:00'

Nobody said you have to use class components. But you have lifecycle methods like componentDidMount and componentWillUnmount

1 Like

I’ve restructured my code as u suggested. I’m completing 27/29 test-cases on run (all except the test case 3 in AUDIO section, has failed in my locally hosted code). test case 13 in TIMER section & test 1 in AUDIO section tends to be fail in CODEPEN. any reason why might this happen? I’ve struggled for one & a half day but still couldn’t able to find a solution. :woozy_face:

@0x74h51N , @lasjorg ;
I’ve tried to complete this project using legacy class-components but couldn’t keep up with passing the test-cases. So, I’ve converted my code into functional-components & currently stuck with passing timer-test-case #1 (it throws an error: expected ‘session’ to equal ‘break’) within my local project (28/29).
In my CODEPEN I’m having multiple test-case failures with unexpected test-case-error results. How can I fix these new issues?

structure changes:

  1. within the App component a useEffect added.
  2. previous ‘UpdateSession’ method was renamed to ‘UpdateDurations’.
  3. ‘ManageSession’ was restructured affect-according to the useEffect.
  4. ‘TimeSet’ sub functional components simplified on passing props. (may not important)

Great, but your code is still too complicated. I see you’ve made your TimeSet component more dynamic with fewer props.

  const [Session,UpdateSession] = React.useState(
    {
      session_time: new Date(1500 * 1000).toISOString().substring(14, 19),
      session_break: 5,
      session_length: 25,
      session_state: "stopped",
      session_paused: false,
      session_type: "session"
    }
  );

However, you should create separate states for session and break lengths, as well as for stop and pause states. This will make it easier to handle and change these states.

But why are you using the Date class? Why?

let local_date = new Date();
    let current_date = local_date.getFullYear()+'-'+
                      (local_date.getMonth()<10 ? '0'+local_date.getMonth() : local_date.getMonth())+'-'+
                      (local_date.getDate()<10 ? '0'+local_date.getDate() : local_date.getDate());
...

Using year or month, why? It’s just minutes and seconds. This doesn’t mean you’re using the Date class well; it just makes your code messier and harder to understand.

Bugs:

  • When you click the reset button, the session length gets set to two and a half minutes. I believe this is because of the complicated date format…
  • When it is in break mode, you can’t pause the clock.

We’ve mentioned this many times, but let’s say it again: use just seconds as an integer for time states please. For example, 5 minutes => 5 * 60 seconds, etc.

1 Like

However, you should create separate…

  • will do.

But why are you using the Date class? Why?

  • when I pass just a seconds value difference (like 1715955763355) and reduced ‘1’ for the updated time, the derived second value passed to the Date obj returns a complete different date-time (time is completely different compared to expected). to prevent this I created this ‘current_date’ & integrate it into a new Date() parameter combining session_break or session_length value. from that I was able to extract the correct minus one time duration.

Using year or month, why? It’s just minutes…

  • sorry my bad, I’m having a hard time adapting to this scenario. I’m a newbie & those(prv understandings) were the scenarios I understood so far.

Bugs: When you click the reset button, the session length gets set…

  • I couldn’t capture this… let me try to capture this.

Bugs: When it is in break mode, you can’t pause the clock.

  • this is not a bug, I created the logic on purpose considering there will be no point of pausing a break-time. it’s just a ‘if statement’ I will remove it if u see it as an unnecessary.

We’ve mentioned this many times, but let’s say it again: use just seconds as …

  • sorry again. I’m hving a hard time converting the seconds back to expected time-frame. this is the main reason I integrated a ‘curren_date’ to a new Date obj & retrieved a desired timeframe (the second err above u pointed-out).

:point_right: sorry for being so bad at coding, I’m doing these things on my own + referencing from tutorials. thanks again for keeping up with me so far!

Don’t be sorry, there’s nothing to be sorry about. Maybe I said it wrong. What I tried to say is that keeping time states as just numbers is easier to manage without date formatting or Date class object. For example, a maximum of 60 minutes means 3600 seconds as an integer. So, manage all times just as integers. That’s all. Also, there’s nothing wrong with making mistakes or bugs. Just try to keep your code simple and understandable. That’s all I was trying to say.

I didn’t know you made break mode not pausable on purpose. I thought it was a bug, but it’s not. Okay then, it’s a good idea.

Also, you’ve learned functional-based React and hooks by yourself, which is great! And yes, you are doing it by yourself with your own ideas, which is great too. :+1:

I believe you will handle this with just a few small adjustments. :ok_hand:

1 Like

Structure Updated:

  • used separate state-changes for each state-values
  const [SessionTime,SetSessionTime] = useState(new Date(15 * 1000).toISOString().substring(14, 19));
  const [SessionBreak,SetSessionBreak] = useState(5);
  const [SessionLength,SetSessionLength] = useState(25);
  const [SessionState,SetSessionState] = useState("stopped");
  const [SessionType,SetSessionType] = useState("countdown");
  const [SessionOnPause,SetSessionOnPause] = useState(false);

  • moved-on with seconds when handling time
  useEffect(() => {
    
    const timer = setInterval(() => {
      if(SessionState==="running" && !SessionOnPause) {

        if(SessionTime.toString()==="00:00") {
          let upd_val = (SessionType==="countdown") ? SessionBreak : SessionLength;
          let upd_time = `${upd_val===60 ? 60 : ((upd_val<10) ? '0'+upd_val : upd_val)}:00`; 
          
          document.getElementById("beep").play();
          
          SetSessionType((SessionType==="countdown") ? "break" : "countdown");  
          SetSessionTime(upd_time);  
        } else {
          let dt = SessionTime.split(':').map(tm => parseInt(tm));
          let mns = (dt[1]-1!==0) ? dt[0] : ((dt[0]===0) ? 0 : dt[0]-1); 
          let scs = (dt[1]-1!==0) ? dt[1]-1 : ((dt[1]-1===0) ? 0 : 59);
          
          setTimeout(() => document.getElementById("time-left").classList.remove("activated"), 1000);
          
          SetSessionTime(new Date(((mns*60) + scs) * 1000).toISOString().substring(14, 19));  
        }
      }
    },1000);

    return () => clearInterval(timer);
  });
  • simplified & restructured ‘ManageSession();’
const ManageSession = (type) {
/* elm class toggle logic in first few lines, then the following */
    if(type==="reset") {
      document.getElementById("beep").pause();
      document.getElementById("beep").currentTime = 0;
      
      SetSessionTime(new Date(1500 * 1000).toISOString().substring(14, 19));
      SetSessionBreak(5);
      SetSessionLength(25);
      SetSessionState("stopped");
      SetSessionType("countdown");
      SetSessionOnPause(false);
    } else if(type==="pause") {
      (SessionType!=="break") && SetSessionOnPause(!SessionOnPause);
    } else {
      SetSessionState("running");
      SetSessionOnPause(false);
    }
  }

Newly encountered errors (18/29 overall success on CODEPEN):

time-left is not formatted correctly: expected ‘01’ to equal ‘60’

Timer has not reached 00:00

  • but it does, double checked even with console.log->ing

Timeout of 100000ms exceeded. For async tests and hooks, ensure “done()” is called; if returning a Promise, ensure it resolves.

  • getting this on multiple test cases & audio-test-case-01

session or break length to <= 0 and >60

  • it does range between 0 to 61 (excluding 0 & 61) but in some test-cases indicate the value became negative

convert all ‘Date() clz objs’ to integers but still getting the errors mentioned above. Am I missing something? :exploding_head:

 const UpdateDurations = (value) => {
    let upd_val = (value["type"]==="break") ? SessionBreak : SessionLength;

    if(value["val"]==="up") {
      upd_val = (upd_val===1) ? 2 : ((upd_val===60) ? 60 : upd_val+1);
    } else {
      upd_val = (upd_val===1) ? 1 : ((upd_val===60) ? 59 : upd_val-1);
    }
        
    if(value["type"]==="break") {
      SetSessionBreak(upd_val);
    } else {
      SetSessionTime(upd_val*60);
      SetSessionLength(upd_val);
    } 
  }
  useEffect(() => {
    
    const timer = setInterval(() => {

      if(SessionState==="running" && !SessionOnPause) {

        if(SessionTime===0) {
          document.getElementById("beep").play();

          setTimeout(() => {
            SetSessionType((SessionType==="countdown") ? "break" : "countdown");  
            SetSessionTime(((SessionType==="countdown") ? SessionBreak : SessionLength)*60);  
          },1000);
        } else {
          setTimeout(() => document.getElementById("time-left").classList.remove("activated"), 1000);
          SetSessionTime(SessionTime-1);  
        }
      }
    },1000);

    return () => clearInterval(timer);
  });
const ManageSession = (type) {
/* elm class toggle logic in first few lines, then the following */
    if(type==="reset") {
      document.getElementById("beep").pause();
      document.getElementById("beep").currentTime = 0;
      
      SetSessionTime(1500); //changed
      SetSessionBreak(5);
      SetSessionLength(25);
      SetSessionState("stopped");
      SetSessionType("countdown");
      SetSessionOnPause(false);
    } else if(type==="pause") {
      (SessionType!=="break") && SetSessionOnPause(!SessionOnPause);
    } else {
      SetSessionState("running");
      SetSessionOnPause(false);
    }
  }
      <p id="time-left" 
        className={(ses_running) ? ((ses_on_countdown) ? "active activated" : "on_break") : "deactivated"}>
          {minutes < 10 ? '0'+minutes : minutes}:{seconds < 10 ? '0'+seconds : seconds}
      </p>

Can you update your codepen with new codes?

sry, I must hv missed it. U can check now, it’s updated. :smiley: