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

task/WMAQA-45 Shared Workspace Tests #23

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

Conversation

shayanaijaz
Copy link
Collaborator

@shayanaijaz shayanaijaz commented Jan 22, 2024

Overview

Added tests for shared workspaces. Also added a teardown project that deals with deleting shared workspaces directly using tapis

Changes

  • Added shared workspace tests which include adding, editing, and searching shared workspaces
  • Added a teardown project. This project runs at the end after all tests are ran. The shared workspace teardown authenticates with tapis using the test users authentication, gets the users shared workspace systems for the specific portal and deletes those systems

Testing

  • Run tests normally using npx playwright test and ensure all tests pass
  • As an addition, also try manually logging onto the portal the tests were ran on and ensure no shared workspaces exist

Notes

In the shared workspace teardown there's a new function page.evaluate being used when doing external http calls. More details about why this is needed and what it does can be found here

Copy link
Collaborator

@fnets fnets left a comment

Choose a reason for hiding this comment

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

There's a couple of bugs I found, but the skip and teardown are working great!

test('Cleanup shared workspaces', async ({ page, baseURL }) => {
const {USERNAME: username, PASSWORD: password} = process.env;

const tenant = 'https://portals.tapis.io';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is ok for now, but we're going to need to parameterize this if we want to test portals on different tenants.

@@ -29,7 +29,8 @@
data = {
"PORTAL_DATAFILES_STORAGE_SYSTEMS": custom_portal_settings._PORTAL_DATAFILES_STORAGE_SYSTEMS or [],
"SYSTEM_MONITOR_DISPLAY_LIST": custom_portal_settings._SYSTEM_MONITOR_DISPLAY_LIST or [],
"NGINX_SERVER_NAME": os.getenv('NGINX_SERVER_NAME')
"NGINX_SERVER_NAME": os.getenv('NGINX_SERVER_NAME'),
"PORTAL_PROJECTS_SYSTEM_PREFIX": custom_portal_settings._PORTAL_PROJECTS_SYSTEM_PREFIX or ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

These or statements don't actually work for me if the key value is missing from the config file. I think you're looking for something like value = <value_if_true> if <expression> else <value_if_false>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The or statements should work. Can you check what python version you have to make sure that isn't an issue?

Copy link
Collaborator

@fnets fnets left a comment

Choose a reason for hiding this comment

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

I think I have one change to make this more secure and another short research task for myself.

}
})

async function getAccessToken(page, username, password, tenant) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this will work, but I think a more secure way to do it would be to use this api:

https://github.com/TACC/Core-Portal/blob/4c210402098f09a7e03a5a739ae4dcf248ce46aa/server/portal/apps/auth/api/urls.py#L5

That way, we're not transmitting passwords or anything. Normally, I'd say we wouldn't want to use the portal to test the portal, but I think in this case it makes sense.


}

async function getSystems(page, tenant, projectPrefix, accessToken) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to delete all the shared systems, or just the one we created in these tests? Is there an easy or free way to get that system id at system creation? I'll take a look to try and save you a headache.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, it looks like it's available in the Shared Workspaces view and in the URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants