Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

control directory for .snap.new files #621

Closed
fowles opened this issue Sep 26, 2024 · 7 comments
Closed

control directory for .snap.new files #621

fowles opened this issue Sep 26, 2024 · 7 comments

Comments

@fowles
Copy link

fowles commented Sep 26, 2024

I am working on a project that uses bazel (which is very strict about hermiticity and directories). In order to make insta play well in this ecosystem, I would like to be able to designate a specific directory that it will write output files to and then to pass that directory to insta review for acceptance.

Roughly something like:

INSTA_NEW_SNAP_DIR=... $TEST_RUNNER
INSTA_NEW_SNAP_DIR=... cargo insta review

Do you think that such a change would be accepted if offered?

@max-sixty
Copy link
Collaborator

Does snapshot_path do this?

insta/insta/src/settings.rs

Lines 480 to 487 in 8753f2c

/// Sets the snapshot path.
///
/// If not absolute it's relative to where the test is in.
///
/// Defaults to `snapshots`.
pub fn set_snapshot_path<P: AsRef<Path>>(&mut self, path: P) {
self._private_inner_mut().snapshot_path(path);
}

@max-sixty
Copy link
Collaborator

I haven't used bazel much myself, but it's also worth checking out INSTA_WORKSPACE_ROOT (and may require CARGO_MANIFEST_DIR until #614 is merged) to set paths across all tests. @thoughtpolice had similar-sounding issues in #575.

Keen to support this sort of setup, so will close the gap if one still exists.

@fowles
Copy link
Author

fowles commented Sep 27, 2024

My current (very hacky) environment has a test:

use insta;

#[test]
fn test_all() {
  insta::with_settings!({snapshot_path => "/Users/beaker/dev/bronto/bronto/tool/e2e/"}, {
    insta::glob!("e2e/*.c", |path| {
      let input = std::fs::read_to_string(path).unwrap();
      insta::assert_snapshot!(input);
    })
  });
}

which when run fails with trying to write the .snap.new file (I think).

$ bazel test //bronto/tool:runner_test --test_env=INSTA_WORKSPACE_ROOT=$(pwd) --test_env=CARGO_MANIFEST_DIR=$(pwd)
WARNING: Build option --test_env has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
INFO: Analyzed target //bronto/tool:runner_test (0 packages loaded, 4556 targets configured).
FAIL: //bronto/tool:runner_test (see /private/var/tmp/_bazel_beaker/66706307a1c7f7ec3458df3c2b2f96d7/execroot/_main/bazel-out/darwin_x86_64-fastbuild/testlogs/bronto/tool/runner_test/test.log)
INFO: From Testing //bronto/tool:runner_test:
==================== Test output for //bronto/tool:runner_test:

running 1 test
test test_all ... FAILED

failures:

---- test_all stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot file: bronto/tool/e2e/[email protected]
Snapshot: [email protected]
Source: bronto/tool/runner.rs:8
Input file: bronto/tool/e2e/id.c
────────────────────────────────────────────────────────────────────────────────
Expression: input
────────────────────────────────────────────────────────────────────────────────
+new results
────────────┬───────────────────────────────────────────────────────────────────
          0 │+int id(int noep) { return noep; }
────────────┴───────────────────────────────────────────────────────────────────
thread 'test_all' panicked at bronto/tool/runner.rs:8:7:
called `Result::unwrap()` on an `Err` value: Os { code: 1, kind: PermissionDenied, message: "Operation not permitted" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I have played around with a bunch of things to help it find the .snap files and can get that to work reliably, however, whenever the test wants to write a .snap.new it fails with a permission error. That is the setting I am trying to control (without much luck).

@max-sixty
Copy link
Collaborator

@fowles I don't get the permission error yet, but I see some other problems. Will have a look. At the very least we should say what file it was trying to write to...

max-sixty added a commit to max-sixty/insta that referenced this issue Oct 1, 2024
@max-sixty
Copy link
Collaborator

@fowles #631 should give us a better error — do you want to try that?

I fixed one other issue. I have a couple of other things I want to look at, but otherwise don't know a precise reason behind the issue you see.

max-sixty added a commit that referenced this issue Oct 1, 2024
@fowles
Copy link
Author

fowles commented Oct 1, 2024

Thanks! I will try this in the next day or two.

@max-sixty
Copy link
Collaborator

I also refactored the code that handles paths, so I would now be quite surprised if INSTA_WORKSPACE_ROOT doesn't work, though not 100% as don't have a test case.

If you find there is still an issue, please reopen / add a new issue and I'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants