Bug #11502

Flaky test on sdk/python/tests/test_arv_get.py

Added by Lucas Di Pentima 8 months ago. Updated 8 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Target version:
Start date:
04/14/2017
Due date:
% Done:

100%

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

Description

======================================================================
FAIL: test_get_collection_manifest (tests.test_arv_get.ArvadosGetTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/1/jenkins/workspace/run-tests-remainder/sdk/python/tests/test_arv_get.py", line 81, in test_get_collection_manifest
    self.assertEqual(self.col_manifest, f.read())
AssertionError: u'. 37b51d194a7513e45b56f6524f2d51f2+3+A9a83a0d33abbdec368acb632ec53e8186758bb3f@5903577a acbd18db4cc2f85cedef654fccc4a4d8+3+Ac1a9a2e550cfe57511b2e70c4dfa1d05a23b75e8@5903577a 0:3:bar.txt 3:3:foo.txt\n./subdir 73feffa4b7f6bb68e44cf984c85f6e88+3+A732d02797529f59e880374c7c91d81be527f3307@5903577a 0:3:baz.txt\n' != '. 37b51d194a7513e45b56f6524f2d51f2+3+A0d0f09d1119414a4ae7d56226f7cc79cd84b834f@5903577b acbd18db4cc2f85cedef654fccc4a4d8+3+A0afbb90b13fb7ccfd24ed7ce5923308dcd9863bb@5903577b 0:3:bar.txt 3:3:foo.txt\n./subdir 73feffa4b7f6bb68e44cf984c85f6e88+3+Aeb981cdb15a3ab7330ff42673b5acd6a92e5489a@5903577b 0:3:baz.txt\n'

Subtasks

Task #11506: Review 11502-unstripped-manifest-fixResolvedLucas Di Pentima

Associated revisions

Revision cbcb0fc8
Added by Lucas Di Pentima 8 months ago

Merge branch '11502-arvget-flaky-test'
Closes #11502

Revision 65e33985
Added by Lucas Di Pentima 8 months ago

Merge branch '11502-unstripped-manifest-fix'
Closes #11502, #11511

History

#1 Updated by Lucas Di Pentima 8 months ago

  • Status changed from New to In Progress

#2 Updated by Lucas Di Pentima 8 months ago

As Peter noted, the comparison on the test should be done with stripped manifests.

#3 Updated by Lucas Di Pentima 8 months ago

Discovered a regression on the new arv-get, when asked to retrieve a manifest using a PDH, its output was a non-stripped manifest.

Fixed the regression and the flaky test at e507e67ee - branch 11502-arvget-flaky-test

Test run: https://ci.curoverse.com/job/developer-run-tests/235/

#4 Updated by Lucas Di Pentima 8 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:cbcb0fc8c3ebc85bf81ba9d50795d62db75efa6c.

#5 Updated by Peter Amstutz 8 months ago

  • Status changed from Resolved to Feedback

#6 Updated by Peter Amstutz 8 months ago

Reviewing the old code for arv-get, the behavior was to call CollectionReader.manifest_text() which is unnormalized/unstripped regardless of whether it is by UUID or PDH.

It is a regression to change that behavior (and it seems to break crunch-job).

A --strip option might be sensible.

#7 Updated by Lucas Di Pentima 8 months ago

Updates at branch 11502-unstripped-manifest-fix - 03188ad6e
Test run: https://ci.curoverse.com/job/developer-run-tests/238/

Reverted change so that asking by PDH (and also UUID), the returned manifest includes its tokens by default
Added --strip-manifest as a new option to write manifests without tokens, as requested.
Updated & added tests.

#8 Updated by Lucas Di Pentima 8 months ago

New updated at 67ba19113
Test run: https://ci.curoverse.com/job/developer-run-tests/240/

As I've unwillingly reintroduced the flaky test, I fixed it by creating new Collection objects from the unstripped manifests, and then compared their stripped manifests to avoid race conditions.
Also made some test code cleanups.

#9 Updated by Tom Clegg 8 months ago

In test_get_collection_unstripped_manifest, how about replacing r"\+A[0-9a-f@]+" with "+Axxxxx" in both expected and actual before comparing, instead of stripping them -- that way the test would ensure arv-get really outputs an unstripped manifest.

Trailing whitespace in sdk/python/arvados/commands/get.py L90

Rest LGTM.

#10 Updated by Lucas Di Pentima 8 months ago

Updates at e7284afa8
Waiting for tests on jenkins to merge to master.

#11 Updated by Lucas Di Pentima 8 months ago

  • Status changed from Feedback to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:65e339856daf4b5c3a4a810cd3a5f1a8e386dc8c.

Also available in: Atom PDF