Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(js-ts): Convert app/util/test/utils.js to TypeScript #11279

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+8 −4
Open

chore(js-ts): Convert app/util/test/utils.js to TypeScript #11279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ecd27be
chore(js-ts): Convert app/util/test/utils.js to TypeScript
devin-ai-integration[bot] Sep 18, 2024
f766924
chore: Remove utils.js after conversion to TypeScript
devin-ai-integration[bot] Sep 18, 2024
387a95a
chore: Remove unnecessary type annotations from utils.ts
devin-ai-integration[bot] Sep 18, 2024
d86e462
chore: Clean up utils.ts and remove notes.md
devin-ai-integration[bot] Sep 18, 2024
e255598
Fix import path for utils.ts in BrowserTab
devin-ai-integration[bot] Sep 23, 2024
c9a5458
Update import paths from utils.js to utils.ts
devin-ai-integration[bot] Sep 24, 2024
5476d4d
commit to trigger CI
devin-ai-integration[bot] Sep 25, 2024
09abb80
Merge remote-tracking branch 'origin/main' into convert-ts-utils
devin-ai-integration[bot] Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/Views/BrowserTab/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ import trackErrorAsAnalytics from '../../../util/metrics/TrackError/trackErrorAs
import { selectPermissionControllerState } from '../../../selectors/snaps/permissionController';
import { useIsFocused } from '@react-navigation/native';
import handleWebViewFocus from '../../../util/browser/webViewFocus';
import { isTest } from '../../../util/test/utils.js';
import { isTest } from '../../../util/test/utils.ts';
import { EXTERNAL_LINK_TYPE } from '../../../constants/browser';

const { HOMEPAGE_URL, NOTIFICATION_NAMES } = AppConstants;
Expand Down
6 changes: 5 additions & 1 deletion app/util/test/utils.js → app/util/test/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ export const flushPromises = () => new Promise(setImmediate);
export const FIXTURE_SERVER_PORT = 12345;

// E2E test configuration required in app
export const testConfig = {};
interface TestConfig {
fixtureServerPort?: number;
}

export const testConfig: TestConfig = {};
Comment on lines +6 to +10
Copy link
Contributor

@Daniel-Cross Daniel-Cross Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really an optional prop? I checked the code base and I can't find fixtureServerPort used anywhere, yet in the function below we reference testConfig.fixtureServerPort ?? 12345. It seems like this will always be 12345 regardless. testConfig doesn't even have a mocked key value pair?

testConfig: {
  fixtureServerPort: 12345
  }

Copy link

@nrdagar nrdagar Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Daniel-Cross We made it optional because of the coalescing operator. Our reasoning was that the below line does have a fallback mechanism if the port is not defined.

testConfig.fixtureServerPort ?? FIXTURE_SERVER_PORT;

Also, usage is there in shim.js. If the launch arguments contain a fixtureServerPort, it's assigned to testConfig.fixtureServerPort. Else, the fallback value is FIXTURE_SERVER_PORT

if (isTest) {
  const raw = LaunchArguments.value();
  testConfig.fixtureServerPort = raw?.fixtureServerPort
    ? raw.fixtureServerPort
    : FIXTURE_SERVER_PORT;
}


/**
* TODO: Update this condition once we change E2E builds to use release instead of debug
Expand Down
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ setupSentry();
import { AppRegistry, LogBox } from 'react-native';
import Root from './app/components/Views/Root';
import { name } from './app.json';
import { isTest } from './app/util/test/utils.js';
import { isTest } from './app/util/test/utils.ts';


import { Performance } from './app/core/Performance';
Expand Down
2 changes: 1 addition & 1 deletion shim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
FIXTURE_SERVER_PORT,
isTest,
testConfig,
} from './app/util/test/utils.js';
} from './app/util/test/utils.ts';
import { LaunchArguments } from 'react-native-launch-arguments';
import { defaultMockPort } from './e2e/mockServer/mockUrlCollection';

Expand Down
Loading