-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
6897f5b
to
0af6603
Compare
tests/id2attrpath.bats
Outdated
# ---------------------------------------------------------------------------- # | ||
|
||
setup_file() { | ||
export DBPATH="$BATS_FILE_TMPDIR/test-cli.sqlite"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear to me what we want our convention to be for this var:
> rg "export DBPATH"
tests/pathological.bats
17: export DBPATH="$BATS_FILE_TMPDIR/test-cli.sqlite";
tests/cli.bats
17: export DBPATH="$BATS_FILE_TMPDIR/test-cli.sqlite";
tests/nixpkgs-flox.bats
43: export DBPATH="$BATS_FILE_TMPDIR/test-nixpkgs-flox.sqlite";
tests/sqlite3.bats
17: export DBPATH="$BATS_FILE_TMPDIR/test.sqlite";
tests/id2attrpath.bats
17: export DBPATH="$BATS_FILE_TMPDIR/test-cli.sqlite";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just use the basement test.sqlite
for all tests for consistency. It's not a real problem to use arbitrary names though.
{ | ||
std::cout << attrPath[i] << " "; | ||
} | ||
std::cout << attrPath[attrPath.size() - 1] << std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if it would be better to use an STL one liner instead of writing this algorithm and realized this will segfault if id2attrpath is called with ID = 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use attrPath.back()
to handle the concern @mkenigs raised. Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle the attrPath.back()
or use nix::concatStringsSep
and you'll be ready to roll.
size_t
is nice too.
Changing the test db names is optional, I don't have strong opinions either way, but feel free to do it in this PR.
tests/id2attrpath.bats
Outdated
# ---------------------------------------------------------------------------- # | ||
|
||
setup_file() { | ||
export DBPATH="$BATS_FILE_TMPDIR/test-cli.sqlite"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just use the basement test.sqlite
for all tests for consistency. It's not a real problem to use arbitrary names though.
tests/id2attrpath.cc
Outdated
return EXIT_FAILURE; | ||
} | ||
|
||
for (unsigned long i = 0; i < attrPath.size() - 1; i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you wrote here works perfectly. There's a shorthand helper that the nix
headers provide if you want something a bit less verbose :
#include <nix/util.hh>
//...
std::cout << nix::concatStringsSep( " ", attrPath ) << std::endl;
( feel free to use either, I just wanted to share a tip )
Co-authored-by: Matthew Kenigsberg <[email protected]>
@zmitchell since you're out for the day I'm going to merge this ( especially the test cases ) into other Thanks for helping out on this! |
This adds a small executable
id2attrpath
with the following interfacethat returns a space-delimited attribute path corresponding to the
AttrSets.id
passed in. Future PRs will extend this toPackages.id
.This program is only intended to make testing easier and shouldn't be used elsewhere.