-
Notifications
You must be signed in to change notification settings - Fork 62
APS Testing [AARD-1941]
#1206
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
base: dev
Are you sure you want to change the base?
APS Testing [AARD-1941]
#1206
Conversation
APS IsSignedIn used to be returning true no matter what. This has no been fixed and APS.isSignedIn() now returns the proper result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* dev: (28 commits) Unused Imports Removed Merge Fix fix(merge): fix merging build issues from GH-1203 Update fission/src/test/MirabufParser.test.ts New Endgame Sound & Updated Licensing Info fix: eslint fix: adding suggested changes Co-authored-by: Azalea Colburn <62953415+azaleacolburn@users.noreply.github.com> fix: comment position changes fix: eslint error fix: moving placement of comment Type & Removed Unnecessary Link Build Fix Documentation refactor: avoid having potentially undefined global functions fix: replacing empty fragment tag with null Cleaner Code Unnecessary Tests Removed fix: formatting feat: hides all touch control schemes if not supported by device More Advanced Joint Testing ...
* dev: fix(snyk): update version requirements reorder dependencies chore(installer): update python packages to avoid vulerabilities fix: clean up workflow spacing, use latest checkout and setup-node steps fix: use correct package.json path feat: add npm caching to all npm-involving workflows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My apologies for merging dev before suggesting these changes. Should have not merged dev till I was ready to approve.
…/synthesis into alexey/1941/APS-Testing
onClick={async () => { | ||
const auth = await APS.getAuth() | ||
auth && APS.refreshAuthToken(auth.refresh_token, true) | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the syntax is a little messed up here. I believe since auth
is a promise it always is evaluated as truthly. For this reason I think an if statement is required but correct me if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auth
isn't a promise here, it's APSAuth | undefined
because APS.getAuth()
is await
ed. It's a bit unconventional to use short-circuiting in JS like this (more common in react components) but this does work
Task
Create Tests for APS.ts
AARD-1941
Symptom
No tests for the APS system
Solution
Add tests for the APS system
Verification
All Unit tests pass
Before merging, ensure the following criteria are met: