Feature #3894

[Documentation] Document the run-tests.sh program on the Hacking pages on the wiki

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

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Documentation
Target version:
Start date:
09/18/2014
Due date:
% Done:

100%

Estimated time:
(Total: 8.50 h)
Story points:
1.0

Subtasks

Task #3930: Fix bugs in run-tests script and make it a bit less hostileResolvedTom Clegg

Task #3936: Review 3894-gem-versionResolvedTom Clegg

Task #3933: Fix pkg_resources.VersionConflict in fuse testsResolvedTom Clegg

Task #3924: Write "Running tests" wiki pageResolvedTom Clegg

Task #3946: Review 3894-improve-run-tests on arvados-devResolvedTom Clegg

Task #3970: Update "making ruby gems" wikiResolvedTom Clegg

Task #3945: Review 3894-gem-version (take 2)ResolvedTom Clegg

Associated revisions

Revision 6fcb589f
Added by Tom Clegg about 5 years ago

Merge branch '3894-gem-version' refs #3894

Revision b25bb303
Added by Tom Clegg about 5 years ago

Merge branch '3894-improve-run-tests' closes #3894

Revision b25bb303
Added by Tom Clegg about 5 years ago

Merge branch '3894-improve-run-tests' closes #3894

Revision fef361d3 (diff)
Added by Tom Clegg about 5 years ago

Fix another -z that should have been -n. refs #3894

Revision fef361d3 (diff)
Added by Tom Clegg about 5 years ago

Fix another -z that should have been -n. refs #3894

History

#1 Updated by Tom Clegg about 5 years ago

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

#2 Updated by Tom Clegg about 5 years ago

  • Category set to Documentation
  • Assigned To set to Tom Clegg

#3 Updated by Tom Clegg about 5 years ago

  • Status changed from New to In Progress

#4 Updated by Radhika Chippada about 5 years ago

Review comments for branch: 3894-gem-version

Tom, looks good. I spent some time trying to understand what magic the " . " is doing in this update. Thanks to your very detailed git log comment that clarified the intent.

#5 Updated by Tom Clegg about 5 years ago

A couple more changes pushed, now at c446dd8

#6 Updated by Brett Smith about 5 years ago

Reviewing arvados-dev 07b0b88

  • A default value for CONFIGSRC is set under if [[ -n "$CONFIGSRC" ]]. Should that check -z instead? Otherwise, it seems like you only override any custom value. You could also flatten this condition with the -d check below it.
    • It would be cool if the script set a default CONFIGSRC early on, then let me override it later. Then I could run it locally with CONFIGSRC= to use the configuration files I already have, without organizing copies. The test in install_apiserver makes me think you had this in mind too.
  • The handling of Python egg_info is inconsistent: the SDK runs it through a specific cut, while FUSE does not. The cut makes it look like you thought about switching the format to use %ci instead of %ct, but then backed that out without reverting the cut. Which is it? (My personal opinion is that we should switch to git log --first-parent --format=format:%ci.%h -n1 . | tr -dc 0-9a-f.. I believe this would be safe, and promote consistency with our Gem versioning. But the only thing required for this story is adding . and keeping them consistent.)

Thanks.

#7 Updated by Tom Clegg about 5 years ago

Brett Smith wrote:

Reviewing arvados-dev 07b0b88

  • A default value for CONFIGSRC is set under if [[ -n "$CONFIGSRC" ]]. Should that check -z instead? Otherwise, it seems like you only override any custom value. You could also flatten this condition with the -d check below it.

Yes, meant -z, thanks. Fixed (with flatten) in 63517ba

  • It would be cool if the script set a default CONFIGSRC early on, then let me override it later. Then I could run it locally with CONFIGSRC= to use the configuration files I already have, without organizing copies. The test in install_apiserver makes me think you had this in mind too.
I added some comments in 8e51498. The behavior I'm hoping for here is
  • If you provide CONFIGSRC=/something on the run-tests.sh command line, config files get copied from there.
  • Otherwise, run-tests.sh doesn't touch your config files.
  • Exception: If $HOME/arvados-api-server/ is a directory, we assume you're ci.curoverse.com and use that as the default CONFIGSRC. (We should probably just update ci and drop the magic default, if the first two options seem reasonable...)

Re "Then I could run it locally..." -- you can, right? (Is this a reference to the -z mistake above, or is there another bug here I'm still not seeing?)

  • The handling of Python egg_info is inconsistent: the SDK runs it through a specific cut, while FUSE does not. The cut makes it look like you thought about switching the format to use %ci instead of %ct, but then backed that out without reverting the cut. Which is it? (My personal opinion is that we should switch to git log --first-parent --format=format:%ci.%h -n1 . | tr -dc 0-9a-f.. I believe this would be safe, and promote consistency with our Gem versioning. But the only thing required for this story is adding . and keeping them consistent.)

Whoops. OK, I forgot to update FUSE. Fixed that in ee10e6b. Also made the change to %ci that I had intended, as you guessed. Used tr -dc instead of a horrible cut list (but added back a small cut to eliminate the 0400 for the time zone).

#8 Updated by Brett Smith about 5 years ago

Tom Clegg wrote:

Re "Then I could run it locally..." -- you can, right? (Is this a reference to the -z mistake above, or is there another bug here I'm still not seeing?)

I sort of missed the purpose of the later -d check. What you describe seems sensible and works for me. Please go ahead and merge this. Thanks.

#9 Updated by Tom Clegg about 5 years ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF