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.

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

feat: drop down of apps from app management #113

Merged
merged 20 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
a944a50
first attempt at adding app names from user's app management, referen…
lenareed Sep 9, 2024
fb00c75
properly loads when click + and history, greys out apps not in app ma…
lenareed Sep 10, 2024
12f4927
removed unnecessary imports/types
lenareed Sep 10, 2024
5be9760
Merge remote-tracking branch 'origin/main' into DEVX-2886
lenareed Sep 10, 2024
1f31469
removed apps without kind, cannot be run because not android or ios a…
lenareed Sep 10, 2024
f70bbdb
moved filter to api call
lenareed Sep 11, 2024
69b9fdf
use vscode native color
lenareed Sep 12, 2024
6fb6d9c
used react to load data
lenareed Sep 12, 2024
501ce47
more informative error, update method names, single type guard for ap…
lenareed Sep 12, 2024
7117b70
fixed dropdown so loads on new window, saves platform kind
lenareed Sep 12, 2024
64c078e
removing log lines
lenareed Sep 12, 2024
792b8f7
moved sort to loadAppNames so saved in state
lenareed Sep 12, 2024
f23b594
filter by explicitly by is_simulator
lenareed Sep 12, 2024
bc38521
fix case issue by making platform name always lowercase
lenareed Sep 12, 2024
f472a10
updated css so grey when visible
lenareed Sep 12, 2024
ff56521
waiting till items to populate dropdown
lenareed Sep 13, 2024
e99dd58
reverted platform type to uppercase platform names, modifying api dat…
lenareed Sep 13, 2024
f2508ad
updating missed platforms from lower case back to uppercase
lenareed Sep 13, 2024
7b7798b
remove unnecessary type declaration
lenareed Sep 14, 2024
47444a3
metadata can be null, move modifying API input to method with fetch
lenareed Sep 16, 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 src/api/llm/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function getHTTPServer(region: Region): string {
return `https://${getDomain(region)}/v1/scriptiq-llm`;
}

function getDomain(region: Region): string {
export function getDomain(region: Region): string {
switch (region) {
case 'staging':
return 'api.staging.saucelabs.net';
Expand Down
29 changes: 27 additions & 2 deletions src/api/llm/http.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { fetch } from 'undici';

import { GlobalStorage } from '../../storage';
import { Credentials, Vote } from '../../types';
import { getHTTPServer } from './config';
import { Credentials, Vote, isAppStorageFilesApiResponse } from '../../types';
import { getHTTPServer, getDomain } from './config';

/**
* Download image from imgURL and save it to `imgDir/imgName`.
Expand Down Expand Up @@ -72,3 +72,28 @@ export async function sendUserRating(
throw new Error('Unexpected status code: ' + resp.status);
}
}

export async function fetchApps(creds: Credentials) {
const response = await fetch(
`https://${getDomain(creds.region)}/v1/storage/files?kind=ios&kind=android`,
{
method: 'GET',
headers: {
'Content-Type': 'application/json',
Authorization: 'Basic ' + btoa(`${creds.username}:${creds.accessKey}`),
},
},
);

if (!response.ok) {
throw new Error('Network response was not ok: ' + response.status);
}

const data = await response.json();

if (isAppStorageFilesApiResponse(data)) {
return data.items;
} else {
throw new Error('Unexpected data format from API: ' + data);
}
}
31 changes: 31 additions & 0 deletions src/editors/TestGenerationPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { generateTest } from '../api/llm/ws';
import { sendUserRating } from '../api/llm/http';
import { executeUpdateHistoryLinksCommand } from '../commands';
import { fetchApps } from '../api/llm/http';

const MAX_HISTORY_LEN = 100;

Expand Down Expand Up @@ -70,6 +71,10 @@
if (TestGenerationPanel.currentPanel) {
// If the webview panel already exists reveal it
TestGenerationPanel.currentPanel._panel.reveal(ViewColumn.One);
if (typeof testID !== 'string') {
// when webview panel already exists, only load names if testID is not a string
TestGenerationPanel.currentPanel.loadApps();
}
} else {
// If a webview panel does not already exist create and show a new one
const panel = window.createWebviewPanel(
Expand All @@ -92,6 +97,9 @@
storage,
);
TestGenerationPanel.currentPanel._testRecordNavigation = true;

// when webview panel doesn't exist, always load app names
TestGenerationPanel.currentPanel.loadApps();
}

if (!TestGenerationPanel.currentPanel._testRecordNavigation) {
Expand Down Expand Up @@ -153,6 +161,29 @@
});
}

private loadApps() {
const creds = this._memento.getCredentials();

if (!creds?.username || !creds?.accessKey || !creds?.region) {
toast.showError(
'Please set your Sauce Labs credentials in the extension settings.',
);
return;
}

fetchApps(creds)
.then((appNames) => {
this._msgQueue.enqueue({
action: 'load-app-names',
data: appNames,
});
})
.catch((error) => {
console.error('Error fetching app names:', error);
toast.showError(`Failed to load app names: ${errMsg(error)}.`);
});
}

/**
* Defines and returns the HTML that should be rendered within the webview panel.
*
Expand Down Expand Up @@ -250,7 +281,7 @@
*/
private _setWebviewMessageListener(webview: Webview) {
webview.onDidReceiveMessage(
async (message: any) => {

Check warning on line 284 in src/editors/TestGenerationPanel.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
switch (message.action) {
case 'ready':
this._msgQueue.ready();
Expand Down Expand Up @@ -564,7 +595,7 @@
}

class MessageQueue {
private _messages: any[];

Check warning on line 598 in src/editors/TestGenerationPanel.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
private _ready: boolean;
private _webview: Webview;

Expand All @@ -574,7 +605,7 @@
this._webview = webview;
}

enqueue(msg: any) {

Check warning on line 608 in src/editors/TestGenerationPanel.ts

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
if (!this._ready) {
this._messages.push(msg);
return;
Expand Down
40 changes: 40 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,43 @@ export function isWebSocketError(data: unknown): data is WebSocketError {
data.type == 'com.saucelabs.scriptiq.error'
);
}

export interface AppInfo {
id: string;
name: string;
kind: string;
metadata: {
is_simulator?: boolean;
};
}

function isAppInfo(data: unknown): data is AppInfo {
return (
typeof data === 'object' &&
data !== null &&
'id' in data &&
typeof data.id === 'string' &&
'name' in data &&
typeof data.name === 'string' &&
'kind' in data &&
typeof data.kind === 'string' &&
'metadata' in data &&
typeof data.metadata === 'object'
);
}

interface AppStorageFilesApiResponse {
items: AppInfo[];
}

export function isAppStorageFilesApiResponse(
data: unknown,
): data is AppStorageFilesApiResponse {
return (
typeof data === 'object' &&
data !== null &&
'items' in data &&
Array.isArray(data.items) &&
data.items.every(isAppInfo)
);
}
5 changes: 5 additions & 0 deletions webview-ui/test-generation/src/App.scss
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
row-gap: 8px;
}

.app-not-loaded,
.app-list:has(.app-not-loaded) {
color: var(--vscode-input-placeholderForeground);
}

footer {
margin: 8px 0 16px 0;
.controls {
Expand Down
75 changes: 42 additions & 33 deletions webview-ui/test-generation/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import { AssertionInput } from './AssertionInput';
import { Preview } from './Preview';
import { AbstractBaseGenerator, AppiumPython, AppiumJava } from './codegen';
import { AppInfo } from '../../../src/types';

function App() {
const [state, dispatch] = useReducer(reducer, initialState);
Expand All @@ -26,7 +27,7 @@
const [showAdditionalSettings, setShowAdditionalSettings] = useState(false);

useEffect(() => {
function handler(event: any) {

Check warning on line 30 in webview-ui/test-generation/src/App.tsx

View workflow job for this annotation

GitHub Actions / test

Unexpected any. Specify a different type
const message = event.data as PostedMessage; // The json data that the extension sent

switch (message.action) {
Expand Down Expand Up @@ -66,6 +67,12 @@
value: message.data,
});
break;
case 'load-app-names':
dispatch({
type: 'loadAppNames',
value: message.data,
});
break;
case 'recover-from-error':
dispatch({
type: 'setGenerationState',
Expand Down Expand Up @@ -115,26 +122,45 @@
status,
steps,
tunnel,
apps,
} = state;

return (
<main>
<section className="inputs">
<h2>What do you want to test?</h2>
<VSCodeTextField
value={appName}
placeholder="From Sauce Labs App Storage, e.g. test.apk"
onInput={(e) => {
if (e.target && 'value' in e.target) {
dispatch({
type: 'setAppName',
value: e.target.value as string,
});
}
}}
>
Application Name
</VSCodeTextField>
<section className="with-label">
<label>App Name</label>
<VSCodeDropdown
onInput={(e) => {
if (e.target && 'value' in e.target) {
const appName: string = e.target.value as string;
const appInfo = apps.find(
(app) => app.name === appName,
) as AppInfo;
dispatch({
type: 'setAppName',
value: {
appName: appName as string,

Choose a reason for hiding this comment

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

const appName: string = e.target.value as string;
is already doing a cast.

platformName: appInfo.kind as 'iOS' | 'Android',

Choose a reason for hiding this comment

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

We are still forcing types onto the compiler though 😅 If there's a bug in

app.kind = app.kind === 'android' ? 'Android' : 'iOS';
, this line will be complicit in the deception of the compiler.

Options

  1. One technique that we use in saucectl quite often, is to massage the data that we receive at the HTTP client level, which would be in fetchApps() in this case. You could return a type of AppInfo that's internal to us, with platformName being a native field of type Platform.
  2. Add a function for type guarding. AppKindToPlatform() or some such. Might result in more 🍝 code over time though.

},
});
}
}}
value={appName}
className="app-list"
>
{appName !== '' &&
!apps.some((appInfo) => appInfo.name === appName) &&
apps.length > 0 &&
!apps.some((appInfo) => appInfo.name === appName) ? (
<VSCodeOption className="app-not-loaded">{appName}</VSCodeOption>
) : null}
{apps.map((appInfo) => (
<VSCodeOption>{appInfo.name}</VSCodeOption>
))}
</VSCodeDropdown>
</section>
<VSCodeTextArea
resize="both"
value={testGoal}
Expand Down Expand Up @@ -186,7 +212,7 @@
}}
value={state.tunnel?.name ?? ''}
>
Tunnel Name
Tunnel Name (optional)
</VSCodeTextField>
<VSCodeTextField
placeholder="Sauce Connect tunnel owner"
Expand All @@ -200,7 +226,7 @@
}}
value={state.tunnel?.owner ?? ''}
>
Tunnel Owner
Tunnel Owner (optional)
</VSCodeTextField>
<VSCodeTextField
value={maxSteps.toString()}
Expand All @@ -223,23 +249,6 @@
>
Cut off steps at
</VSCodeTextField>
<section className="with-label">
<label>Platform</label>
<VSCodeDropdown
onInput={(e) => {
if (e.target && 'value' in e.target) {
dispatch({
type: 'setPlatformName',
value: e.target.value as 'iOS' | 'Android',
});
}
}}
value={platform.name}
>
<VSCodeOption>Android</VSCodeOption>
<VSCodeOption>iOS</VSCodeOption>
</VSCodeDropdown>
</section>
<VSCodeTextField
onInput={(e) => {
if (e.target && 'value' in e.target) {
Expand Down
49 changes: 37 additions & 12 deletions webview-ui/test-generation/src/state.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { v4 as uuidv4 } from 'uuid';
import { Credentials, TestRecord, Vote } from '../../../src/types';
import { Credentials, TestRecord, Vote, AppInfo } from '../../../src/types';

export interface Assertion {
value: string;
Expand Down Expand Up @@ -78,6 +78,8 @@ export interface State {
language: 'python' | 'java';

tunnel: Tunnel;

apps: AppInfo[];
}

export const initialState: State = {
Expand All @@ -102,14 +104,20 @@ export const initialState: State = {
name: '',
owner: '',
},
apps: [],
};

export type Action =
| { type: 'clear' }
| { type: 'setAppName'; value: State['appName'] }
| {
type: 'setAppName';
value: {
appName: State['appName'];
platformName: State['platform']['name'];
};
}
| { type: 'setTestGoal'; value: State['testGoal'] }
| { type: 'setMaxSteps'; value: string }
| { type: 'setPlatformName'; value: State['platform']['name'] }
| { type: 'setPlatformVersion'; value: State['platform']['version'] }
| { type: 'setStatus'; value: State['status'] }
| { type: 'setGenerationState'; value: State['generationState'] }
Expand All @@ -118,6 +126,7 @@ export type Action =
| { type: 'setTunnelOwner'; value: State['tunnel']['owner'] }
| { type: 'showTestRecord'; value: { testRecord: TestRecord; votes: Vote[] } }
| { type: 'loadNewRecord'; value: TestRecord }
| { type: 'loadAppNames'; value: AppInfo[] }
| { type: 'addAssertion'; value: { key: string } }
| { type: 'removeAssertion'; value: { key: string } }
| { type: 'setAssertionValue'; value: { key: string; value: string } }
Expand Down Expand Up @@ -247,7 +256,11 @@ export const reducer = (current: State, action: Action): State => {
case 'setAppName':
return {
...current,
appName: action.value,
appName: action.value.appName,
platform: {
...current.platform,
name: action.value.platformName,
},
};
case 'setTestGoal':
return {
Expand All @@ -263,14 +276,6 @@ export const reducer = (current: State, action: Action): State => {
maxSteps,
};
}
case 'setPlatformName':
return {
...current,
platform: {
...current.platform,
name: action.value,
},
};
case 'setPlatformVersion':
return {
...current,
Expand Down Expand Up @@ -360,6 +365,26 @@ export const reducer = (current: State, action: Action): State => {
},
};
}
case 'loadAppNames': {
let apps = action.value;
apps = apps.filter(
(item) =>
item.metadata === null ||

Choose a reason for hiding this comment

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

I'm a wee bit confused, since according to the AppInfo interface, metadata is always defined. Which is it?

Copy link
Contributor Author

@lenareed lenareed Sep 16, 2024

Choose a reason for hiding this comment

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

Since metadata might be null, though rare, I'll allow metadata to be null in the type.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd vote for adding it to the interface. The downside is a few extra null checks, upside is an accurate and reliable type.

!('is_simulator' in item.metadata) ||
item.metadata.is_simulator === false,

Choose a reason for hiding this comment

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

Can be simplified:

Suggested change
!('is_simulator' in item.metadata) ||
item.metadata.is_simulator === false,
!item.metadata.is_simulator

);
apps.forEach((app) => {
app.kind = app.kind === 'android' ? 'Android' : 'iOS';

Choose a reason for hiding this comment

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

See my other comments w/r/t/ massaging the data at the HTTP client level or type guarding.

});
if (apps) {
apps.sort((a, b) => a.name.localeCompare(b.name));
}

return {
...current,
apps: apps,
};
}
case 'showTestRecord': {
const { testRecord, votes } = action.value;
let { user_screen_descs = [''] } = testRecord;
Expand Down
Loading
Loading