[GitHub] [superset] eric-briscoe commented on a diff in pull request #22457: fix(dashboard): Add runtime safety checks and improved tests

[GitBox <gi...@apache.org>]

eric-briscoe commented on code in PR #22457:
URL: https://github.com/apache/superset/pull/22457#discussion_r1063859244


##########
superset-frontend/src/dashboard/util/findParentId.test.ts:
##########
@@ -41,4 +41,15 @@ describe('findParentId', () => {
   it('should return null if the parent cannot be found', () => {
     expect(findParentId({ childId: 'a', layout })).toBeNull();
   });
+
+  it('should not throw error and return null with bad / missing inputs', () => {
+    // @ts-ignore
+    expect(findParentId(null)).toBeNull();
+    // @ts-ignore
+    expect(findParentId({ layout })).toBeNull();
+    // @ts-ignore
+    expect(findParentId({ childId: 'a' })).toBeNull();
+    // @ts-ignore

Review Comment:
   @eschutho those are intentional because I am passing invalid data in the test file on purpose to simulate bad data creeping in somewhere in process of calling this function.  So if I change the tests to not have type errors, I cannot test bad inputs.  Because we have a mix of ts and regular js code, and we don't have consistent validation of API data and data mutations through redux and components, the TypeScript I added won't solve the issue where the function call still get called with bad data.  Does this make sense?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org