Arvados

Per discussion in the office: 5214183 has been updated to use more descriptive variable names in the test, to make it clearer what's being exercised.

Review feedback. Some of the items I listed below may be out of scope / too hard to do / do not add much value; please ignore any such items. Thanks.

• Thank you so much for “naming” the variables (shared_dirs, fuse_user_dirs etc) and the comments. It greatly helped understand what the test is doing.

• Would you please rename “collection_owned_by_fuse_1” and “collection_owned_by_fuse_2” as “collection_1_owned_by_fuse” and “collection_2_owned_by_fuse”. Current naming makes them sound like these are collection(s) owned by “fuse_1” and “fuse_2”.

• Can you add test fixtures for pipeline instances for fuse user and look for it? The older version of the test had them.

• The old test also had a link verification for fuse user for what it’s worth. May be you can add fuse user to "all users" group (link) and verify it?

• While at it, can you also add a pipeline instance in fuse user project and look for it?

• May be a link verification also for the fuse user project?

• Would it be possible to verify the details for some of the other objects as well (as in line 282 of test_mount.py)? I think it would be nice to check pipeline instance name and associated template uuid, and manifest text for a collection.

• It appears that both collection_owned_by_fuse_1 and collection_owned_by_fuse_2 are very similar in manifest text content. Would it be possible to use slightly more complex manifest text with directories for the second one (similar to collection_with_files_in_subdir)? And, if possible, verify this text in the test (similar to line 282).

• I think it would be nice if we could have a sub-project under the fuse user project and look for it. (I am not sure if it is easy enough to do or not. Just a suggestion.)

• I am wondering if it would make sense to add a permission link to a shared project and look for it's contents? Of course, this might mean that we do not reuse any existing shared project and instead we add a new shared project fixture with one or two objects in it so that it stays static (which is the main problem we are addressing in this update). May be a stretch.

• I know it is out of scope for this update and I am adding this just as an observation. There are still many d1, d2, ... in the "test_mount.py" and it would be awesome if we could some day comb through this file and update all of them.

New revision at f39bdf8

Radhika Chippada wrote:

Review feedback. Some of the items I listed below may be out of scope / too hard to do / do not add much value; please ignore any such items. Thanks.

• Thank you so much for “naming” the variables (shared_dirs, fuse_user_dirs etc) and the comments. It greatly helped understand what the test is doing.

• Would you please rename “collection_owned_by_fuse_1” and “collection_owned_by_fuse_2” as “collection_1_owned_by_fuse” and “collection_2_owned_by_fuse”. Current naming makes them sound like these are collection(s) owned by “fuse_1” and “fuse_2”.

OK, done.

• Can you add test fixtures for pipeline instances for fuse user and look for it? The older version of the test had them.

Added pipeline_instance_owned_by_fuse.

• The old test also had a link verification for fuse user for what it’s worth. May be you can add fuse user to "all users" group (link) and verify it?

Sure, added an empty collection link owned by the FUSE user (it seems reasonable to test specifically links that the user owns).

• While at it, can you also add a pipeline instance in fuse user project and look for it?

Added pipeline_instance_in_fuse_project.

• May be a link verification also for the fuse user project?

Tricky. This test authenticates as user admin, so projects that are simply shared with the FUSE user aren't exposed directly in this interface.

It seems like it would be a good idea to rewrite this test, or write an additional one, to authenticate as user fuse, so the test can evaluate how the filesystem is presented to the unprivileged user. But that's out of scope for this ticket.

• Would it be possible to verify the details for some of the other objects as well (as in line 282 of test_mount.py)? I think it would be nice to check pipeline instance name and associated template uuid, and manifest text for a collection.

• It appears that both collection_owned_by_fuse_1 and collection_owned_by_fuse_2 are very similar in manifest text content. Would it be possible to use slightly more complex manifest text with directories for the second one (similar to collection_with_files_in_subdir)? And, if possible, verify this text in the test (similar to line 282).

• I think it would be nice if we could have a sub-project under the fuse user project and look for it. (I am not sure if it is easy enough to do or not. Just a suggestion.)

• I am wondering if it would make sense to add a permission link to a shared project and look for it's contents? Of course, this might mean that we do not reuse any existing shared project and instead we add a new shared project fixture with one or two objects in it so that it stays static (which is the main problem we are addressing in this update). May be a stretch.

These certainly seem like reasonable tests. Can we put them in a separate story to test FUSE functionality more thoroughly? I don't mind doing the work, but this was supposed to be a quick ticket to get Jenkins to stop complaining every time we meddle with the active user test fixtures, so I'd like to get it out of the way :-)

• I know it is out of scope for this update and I am adding this just as an observation. There are still many d1, d2, ... in the "test_mount.py" and it would be awesome if we could some day comb through this file and update all of them.

That's certainly reasonable and shouldn't be hard. I don't mind doing that if it doesn't make the diffs too noisy for you. :-)

Added #4387 to track some of the tests suggested in this ticket.

With that said (#4387), LGTM