Fix chartjs warnings

[tsmock]

See #5721 (comment) for additional context.

Big things:

• Keep current behavior as much as possible
• ISO 8601 YYYY-MM-DD for chart X axis labels
• Tooltips shouldn't show hour/minute/second
• I ended up expanding the tooltips a bit ("Thursday, July 6, 2023" instead of "2023-07-06").

[tsmock]

That is funny. It wasn't giving me that warning when I was working on it. And it is also erroring out for me too. Which makes me think I forgot to run yarn install or something.

Anyway, I'll see if I can fix this today.

[tsmock]

It should be fixed now. When I was working on this, chartjs was sending the tick value instead of the tick label to the calllback, and something changed somewhere so that the tick label was sent instead of the tick value. So instead of something like "Jun 1" being sent to the callback, I was getting 1685599200000. So I must have been using an outdated/borked version of a dependency somewhere.

[HelNershingThapa]

Getting the following error now:

./node_modules/chart.js/dist/chart.js 559:18
Module parse failed: Unexpected token (559:18)
File was processed with these loaders:
 * ./node_modules/react-scripts/node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
| };
| class DatasetController {
>   static defaults = {};
|   static datasetElementType = null;
|   static dataElementType = null;

[tsmock]

It looks like I had some stuff from #5908. Its probably the chartjs update that did that. I'll back that out and check and see if it works properly.

[tsmock]

It should be fine now (tested with rm -rf node_modules && yarn install --check-files && yarn build && yarn start).

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.1% Duplication

[HelNershingThapa]

Thank you for fixing those warnings. I like how the tooltip format has been modified; it provides a more user-friendly touch. Although I personally preferred the display of months instead of YYYY-MM-DD, I appreciate the changes nonetheless.

[tsmock]

preferred the display of months instead of YYYY-MM-DD

I can see it going either way. With that said, the test project I was using had 3 tasks mapped on different days in one month, and then a task mapped in that same month the next year. So the labels were June 2023 ---- June 2023 ---- June 2023 ---- June 2024. Which made no sense to me, so I changed it back to the YYYY-MM-DD format (which is where the ticks.callback function comes from).

[HelNershingThapa]

CircleCI is consistently failing (9 times) because of a failing test case with src/views/tests/project.test.js (13.625 s) ● Project Detail Page › should render component details, so I've been holding this off from being merged.

[tsmock]

FML. It worked both in the IDE and on the command line for me. I'll try multiple runs and see if it appears so I can debug it. :(

EDIT: It took 16 runs on the command line until it reproduced, "FAIL src/views/tests/taskSelection.test.js (34.597 s) \r\n ● Task Selection Page › should change the button text to map selected task when user selects a task"

I bet this is a timing issue or something stupid like that. :(

[HelNershingThapa]

I completely understand your frustration. This is the same test case issue we've been battling for a while now. In the past, CircleCI builds would typically pass, but unfortunately, this time, it didn't.

Debugging this issue has been a headache for me as well. I think I'll take a shot at fixing warnings in src/views/tests/project.test.js in the hope that they magically start passing.

[tsmock]

If you are looking at fixing the forwardRef warnings, see 0de7dd2 and (maybe, I don't remember if this actually fixed it) cd758ca for the memory leak issue.

[HelNershingThapa]

I had no intention of repairing those. I was aware that you had handled it on the larger PR, so I was looking for other warnings in 930545c . My attempt was futile; I split off from this PR and incorporated #5979 in another branch, but the failing test still didn't pass.

[willemarcel]

@HelNershingThapa @ramyaragupathy When I commented out this line, the time spent running the test decreased from 599 to 119ms. So Maybe it could be a good solution, as you're testing if the user can see the project or not.

[HelNershingThapa]

Indeed @willemarcel, commenting out that particular line does improve the test execution time significantly and allows the test case to pass. It makes sense since the line checks the visibility of a comment in the "questions and comments" section.

After implementing this workaround, CircleCI encountered another similar issue. Another test case is now failing due to the same "timeout exceeded" problem (more info here) 🤯.

[willemarcel]

@HelNershingThapa this await doesn't look correct: https://github.com/hotosm/tasking-manager/blob/develop/frontend/src/components/teamsAndOrgs/tests/tasksStats.test.js#L28

[HelNershingThapa]

@willemarcel looks like so; it's using expect without an assertion. Thank you for catching that! I'll revise it and remove the expect function to see if it resolves the problem.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
3 Security Hotspots
14 Code Smells

No Coverage information
0.2% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
28 Code Smells

No Coverage information
0.2% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

[tsmock]

I just ran a bunch of tests on the develop branch and this branch.

The develop branch tests were run on a mac (100% pass rate) and on a linux box (~50% pass rate). The pass rate for the chartjs branch was a bit worse on the linux box (~33%).

The most common tests to "fail" were in src/views/tests/taskSelection.test.js (~66%), but I suspect that the failing tests are generally flaky. I'm going to run a while loop overnight, but I suspect that all of the tests that failed in the chartjs branch will also fail in the develop branch, just at lower rates.

[tsmock]

Logs from running for several days (linux)
develop-tasks.logs.tar.gz
develop-mac-tasks.logs.tar.gz
chartjs-tasks.logs.tar.gz

[tsmock]

I've looked for failing test files from the logs in the previous comment. Both of the linux runs were on the same box, run at the same time. I'll note that there are some cancellations counted in all environments (ctrl+c) due to OOM issues.

Failing test files

File	chartjs (linux, 394 runs)	develop (linux, 385 runs)	develop (mac, 1029 runs)
src/components/banner/tests/topBanner.test.js	1
src/components/contributions/tests/myProjectsDropdown.test.js			1
src/components/header/tests/notificationBell.test.js	1
src/components/homepage/tests/stats.test.js		1
src/components/notifications/tests/notificationBodyCard.test.js	1
src/components/projectDetail/tests/questionsAndComments.test.js			2
src/components/projectDetail/tests/similarProjects.test.js		1	1
src/components/projects/tests/moreFiltersForm.test.js			2
src/components/taskSelection/tests/contributions.test.js	6	5
src/components/taskSelection/tests/action.test.js			3
src/components/taskSelection/tests/actionSidebars.test.js			1
src/components/taskSelection/tests/contributions.test.js			6
src/components/taskSelection/tests/index.test.js			9
src/components/taskSelection/tests/lockedTasks.test.js	2	1
src/components/taskSelection/tests/resourcesTab.test.js			1
src/components/taskSelection/tests/taskList.test.js	5	5	14
src/components/teamsAndOrgs/tests/members.test.js	1
src/views/tests/management.test.js	2		3
src/views/tests/organisationManagement.test.js			4
src/views/tests/project.test.js	1		2
src/views/tests/taskAction.test.js			4
src/views/tests/taskSelection.test.js	33	27	30
src/views/tests/teams.test.js			5
src/views/tests/userDetail.test.js			5
src/views/tests/users.test.js	4		1
Totals	57	40	94

The absolute flakiest test file is src/views/tests/taskSelection.test.js.

[tsmock]

OK. It has taken me awhile, but I've figured out what is causing the test failures.

React.lazy.

In order to understand why React.lazy is a problem, we must first understand how jest starts and times tests.
Specifically, jest will load all files required (so a file importing foo importing bar will load the file + foo + bar before starting the tests). I don't know why it does the imports first, but the effect of doing so is that it means that the time of loading the imports is used against the tests.
By contrast, imports loaded via React.lazy are loaded from disk dynamically, and are counted against the time for the test.

In other words,

1. jest loads test file
2. jest recursively resolves imports for the test file
3. jest starts tests
4. For each test
   1. Start a timer
   2. Run the test
      • If React.lazy is used somewhere, the additional imports required are resolved as the code executes.
   3. Stop the timer, report if the test took too long

So, how can we work around this import timing problem?

1. We can import the slow module in the test itself -- in this case, we must document why we have an unused import.
2. We can mock the slow module

Of the two problems, (1) is probably better, since it should be more reliable (AKA: we don't have to keep mocks in sync with code).

[sonarcloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[manjitapandey]

The graph on manage statistics page has differ from previous. The graphs previously shows adjacent individual pillars for tasks mapped and validated. However, the new one shows both mapped and validated tasks on single pillar.

cc: @ramyaragupathy , @royallsilwallz

[tsmock]

It looks like this was a bug; the original definitions (see #4103) for the graphs were

    scales: {
      yAxes: [
        {
          stacked: true,
          ticks: {
            beginAtZero: true,
          },
        },
      ],
      xAxes: [
        {
          stacked: true,
        },
      ],
    },

If we want to keep the second style (side-by-side), we need to drop stacked: true from the axis definitions in tasksStatsChart.js.