Add fixtures_path in sqlx::test args

[ripa1995]

Motivation
In big projects, with a lot of integration tests, it happens that the same fixture needs to be used in multiple tests which are structured in sub folders, leading to duplicated ./fixtures/ files.

Changes
This PR adds fixtures_path, allowing to use fixtures outside the static path used by fixtures (i.e., fixtures/{file_name}.sql).
E.g., #[sqlx::test(fixtures_with_path("../other_dir/fixtures/users"))].
Currently it is accepted only one between fixtures and fixtures_path, like for migrations and migrator. This mostly to avoid issues with the execution order of the fixtures themselves.

Related Issues
Closes #2539

[ripa1995]

@abonander have you got time to check this one? Let me know if any change is needed. Thanks!

[EamonHeffernan]

It would be beneficial to be able to provide a string relative to the root of the project so that the relative position doesn't need to be changed depending on the folder of the test. Alternatively being able to provide a path via an ENV var would be helpful in order to avoid rewriting the fixture names each time.

[ripa1995]

It would be beneficial to be able to provide a string relative to the root of the project so that the relative position doesn't need to be changed depending on the folder of the test. Alternatively being able to provide a path via an ENV var would be helpful in order to avoid rewriting the fixture names each time.

Umh, this could be nice, through the ENV var only probably, because if done through CARGO_MANIFEST_DIR for example, then its value changes depending on the folder in which you are running the cargo test command, and in projects where you have sub-packages then it may cause some unexpected behaviour.

Furthermore, an ENV variable could be applied to the standard fixtures arg.

@abonander, what do you think?

[abonander]

Sorry for the delay. When I wrote this comment, I didn't fully grasp the existing design of this PR nor did I properly explain what I had in mind.

I think there should be a few different operating modes that should cover most use cases:

• The current behavior, which references files in ./fixtures relative to the current file:

// Executes `./fixtures/users.sql` then `./fixtures/posts.sql`
#[sqlx::test(fixtures("users", "posts"))]

• Fixture names should also be allowed to be paths, in which case they can either be absolute, or relative to the current file:

// Executes `../fixtures/users.sql` then `../fixtures/posts.sql`
#[sqlx::test(fixtures("../fixtures/users.sql", "../fixtures.posts.sql"))]

Note that the .sql extensions should be required in order to make this distinct from the existing behavior.
I would probably forbid mixing and matching this and the previous style in the same attribute to prevent confusion.

• An additional parameter, fixtures_path, which changes the relative path of the first style (this should forbid the use of relative paths for the fixture names as that would get confusing):

// Executes `../fixtures/users.sql` then `../fixtures/posts.sql`
#[sqlx::test(
    fixtures_path = "../fixtures",
    fixtures("users", "posts")
)]

For both the second and third style, we can define a placeholder variable that fills in the path of the workspace root, so multiple crates in the same workspace can share fixtures:

// Executes `$WORKSPACE/fixtures/users.sql` then `$WORKSPACE/fixtures/posts.sql`
#[sqlx::test(
    fixtures("$WORKSPACE/fixtures/users.sql", "$WORKSPACE/fixtures/posts.sql")
)]

// Executes `$WORKSPACE/fixtures/users.sql` then `$WORKSPACE/fixtures/posts.sql`
#[sqlx::test(
    fixtures_path = "$WORKSPACE/fixtures",
    fixtures("users", "posts")
)]

I suppose this could be environment variable expansion, but $WORKSPACE would require special handling as Cargo doesn't provide anything like it automatically.

Some alternative syntaxes for the third style (open to opinions here):

// All of these execute `../fixtures/users.sql` then `../fixtures/posts.sql`
#[sqlx::test(
    fixtures(
       // This feels... naked.
       path = "../fixtures",
       "users", 
       "posts"
    )
)]

#[sqlx::test(
    fixtures(
       // This seems to imply "include all fixtures in this directory"
       path("../fixtures"),
       "users", 
       "posts"
    )
)]

#[sqlx::test(
    fixtures(
        path = "../fixtures",
        // Better encapsulated, but not sure about naming this `files`
        files("users", "posts")
    )
)]

Having the path inside fixtures() means we could technically make this composable, e.g.:

// Executes, in order:
// `$WORKSPACE/fixtures/users.sql`
// `$WORKSPACE/fixtures/posts.sql`
// `./fixtures/comments.sql`
#[sqlx::test(
    fixtures(
       path("$WORKSPACE/fixtures"),
       "users", 
       "posts"
    ),
    fixtures("comments")
)]

Although I feel like that's starting to make the feature a little too complicated for its own good.

[abonander]

See: #2545 (comment)

(wish I could mark an existing comment for this)

[ripa1995]

Sorry for the delay. When I wrote this comment, I didn't fully grasp the existing design of this PR nor did I properly explain what I had in mind.

I think there should be a few different operating modes that should cover most use cases:

• The current behavior, which references files in ./fixtures relative to the current file:

// Executes `./fixtures/users.sql` then `./fixtures/posts.sql`
#[sqlx::test(fixtures("users", "posts"))]

• Fixture names should also be allowed to be paths, in which case they can either be absolute, or relative to the current file:

// Executes `../fixtures/users.sql` then `../fixtures/posts.sql`
#[sqlx::test(fixtures("../fixtures/users.sql", "../fixtures.posts.sql"))]

Note that the .sql extensions should be required in order to make this distinct from the existing behavior. I would probably forbid mixing and matching this and the previous style in the same attribute to prevent confusion.

I've implemented the first two types of operating modes, which can be combined with multiple fixtures parameters. Please note that I've decided to not require the user to put .sql to differentiate from the implicit path operating mode, instead, I'm actually checking if the arguments inside a fixtures parameter starts or not with ./ or ../, which defines an explicit path operating mode.

Regarding the third operating mode, I would go with the following:

#[sqlx::test(
fixtures(
path = "../fixtures",
// Better encapsulated, but not sure about naming this files
files("users", "posts")
)
)]

regarding the naming, maybe scripts is better?

For now, I would avoid the use of a placeholder variable.

[abonander]

Please note that I've decided to not require the user to put .sql to differentiate from the implicit path operating mode,

I strongly disagree, but perhaps I was a bit ambiguous with my wording.

We should require the path to actually match what it is on the filesystem to make it more distinct and limit confusion. We shouldn't be adding anything to it, or removing anything from it.

If the file exists as ../foo/bar/baz.sql relative to the current source file, the user should have to specify ../foo/bar/baz.sql. Otherwise, it's very weird that it looks like a path and behaves like a path, but is not a path. Plus, people are likely going to forget and type the extension anyway, then have to deal with confusing errors like could not find file /home/user/project/src/foo/bar/baz.sql.sql.

In short, it would violate the principle of least astonishment.

I don't care if the path actually ends with .sql or not. That's just a convention. If the actual file doesn't have an extension then we shouldn't require the user to add one.

Regarding the third operating mode, I would go with the following:

That sounds good to me. scripts is better than anything I can think of, let's go with it.

[ripa1995]

Please note that I've decided to not require the user to put .sql to differentiate from the implicit path operating mode,

I strongly disagree, but perhaps I was a bit ambiguous with my wording.

We should require the path to actually match what it is on the filesystem to make it more distinct and limit confusion. We shouldn't be adding anything to it, or removing anything from it.

If the file exists as ../foo/bar/baz.sql relative to the current source file, the user should have to specify ../foo/bar/baz.sql. Otherwise, it's very weird that it looks like a path and behaves like a path, but is not a path. Plus, people are likely going to forget and type the extension anyway, then have to deal with confusing errors like could not find file /home/user/project/src/foo/bar/baz.sql.sql.

In short, it would violate the principle of least astonishment.

I don't care if the path actually ends with .sql or not. That's just a convention. If the actual file doesn't have an extension then we shouldn't require the user to add one.

Ok now I got it, thanks for explaining more in details.

To recap, now there are 3 styles that can be used inside a fixtures argument.

• First style, i.e. relative path (old style):

// Executes `./fixtures/users.sql` then `./fixtures/posts.sql`
#[sqlx::test(fixtures("users", "posts"))]

• Second style, i.e. explicit path (requires to specify .sql extension):

// Executes `../fixtures/postgres/users.sql` then `../fixtures/postgres/posts.sql`
#[sqlx::test(fixtures("../fixtures/postgres/users.sql", "../fixtures/postgres/posts.sql"))]

• Third style, i.e. custom relative path:

// Executes `../fixtures/postgres/users.sql` then `../fixtures/postgres/posts.sql`
#[sqlx::test(fixtures(path = "../fixtures/postgres", scripts("users", "posts")))]

The above can be combined using multiple fixtures arguments, e.g.:

// Executes, in order:
// `../fixtures/postgres/users.sql`
// `../fixtures/postgres/posts.sql`
// `./fixtures/comments.sql`
// `../fixtures/postgres/users_bis.sql`
#[sqlx::test(
    fixtures(
       path = "../fixtures/postgres",
       scripts("users", "posts")
    ),
    fixtures("comments"),
    fixtures("../fixtures/postgres/users_bis.sql"
)]

If that works for you, I'll update the documentation as final step.

[abonander]

Second style, i.e. explicit path (requires to specify .sql extension):

In this case I don't want to require that the extension be .sql. I want to give users an option to use a different file extension if they wish.

Instead, I would detect this case by seeing if it looks like a relative (or absolute) path:

let path = Path::new(path_str);

// This will be `true` if there's at least one path separator (`/` or `\`)
// It's also true for all absolute paths, even e.g. `/foo.sql` as the root directory is counted as a component.
let is_explicit_path = path.components().count() > 1;

Also, before adding .sql to the implicit case, I would check if the filename already has an extension:

let has_extension = Path::new(path_str).extension().is_some();

[ripa1995]

Done. I've also updated the docs accordingly.

Let me know if you have any other comment, and thanks for your time!

[abonander]

Looks good, thanks!