Bug #4040

[Tests] FUSE mount test suite should not fail just because new collection fixtures have been added

Added by Tom Clegg almost 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Tim Pierce
Category:
Tests
Target version:
Start date:
10/29/2014
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Subtasks

Task #4351: Review 4040-fuse-testsResolvedRadhika Chippada

Task #4207: Add fixtures exclusively for testing FUSE mountsResolvedTim Pierce


Related issues

Has duplicate Arvados - Bug #4102: [Keep] Fix FUSE test suite so it does not need to be updated every time a new collection fixture is added to apiserverRejected

Associated revisions

Revision 313f5fed
Added by Tim Pierce almost 5 years ago

Merge branch '4040-fuse-tests'

Fixes #4040.

History

#1 Updated by Ward Vandewege almost 5 years ago

  • Target version changed from Arvados Future Sprints to 2014-10-29 sprint

#2 Updated by Tim Pierce almost 5 years ago

  • Assigned To set to Tim Pierce

#3 Updated by Tim Pierce almost 5 years ago

  • Target version changed from 2014-10-29 sprint to Arvados Future Sprints

#4 Updated by Tim Pierce almost 5 years ago

  • Target version changed from Arvados Future Sprints to 2014-11-19 sprint

#5 Updated by Tim Pierce almost 5 years ago

  • Subject changed from [Keep] FUSE mount test suite should not fail just because new collection fixtures have been added to [Tests] FUSE mount test suite should not fail just because new collection fixtures have been added
  • Category set to Tests

#6 Updated by Tim Pierce almost 5 years ago

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.

#7 Updated by Radhika Chippada almost 5 years ago

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.

#8 Updated by Tim Pierce almost 5 years ago

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. :-)

#9 Updated by Tim Pierce almost 5 years ago

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

#10 Updated by Radhika Chippada almost 5 years ago

With that said (#4387), LGTM

#11 Updated by Tim Pierce almost 5 years ago

  • Status changed from New to Resolved

Applied in changeset arvados|commit:313f5fedd4214d077e2b5c7c26bab4df3895c44a.

Also available in: Atom PDF