fix: fix snapshot tests #277
Conversation
|
Don't have windows to debug, seems like it can't find a dll maybe the pcap sdk ? |
|
Huge thanks for fixing the tests. This was giving me a real headache since I have no clue how it all works.
Yeah I think you are correct. If you take a look at build.rs, pcap has to be explicitly downloaded for bandwhich to compile at all. I suspect we might have to do something similar for the tests, either in the tests themselves or in CI. I'm not too familiar with Windows development either, so I'll go dig around a bit. |
|
Alright, I tested it on my Windows VM. In a graphical environment, there's an error popup that says "packet.dll was not found". So yeah, the user needs to have npcap (the binary release!!, not the SDK) installed to run the tests. But it gets more complicated for CI because only the paid OEM version offers silent install, which is what we need[1] here. Apparently others have run into the same issue (nmap/npcap#227), and it seems like npcap developers are willing to offer their OEM installers to open source projects for CI purposes. So I guess I'll be shooting them an email for this. [1]: Since a Windows installer executable is just an archive, technically I can cheat and extract the DLL we need manually from the installer of the free version. But obviously that's the last-ditch solution. |
|
I'll merge this for now, since that Windows CI fix can be its separate thing. Thanks a lot for the contribution. |
Not sure how it worked before, but the default location of snapshots is "./snapshots" but it currently is inside"../snapshots"
There are 2 ways to fix this, move to the correct folder : this is whats done in this pr, the second way is to use https://docs.rs/insta/latest/insta/struct.Settings.html#method.set_snapshot_path (which should result a way smaller diff) but I'm not sure where to call that
cargo-insta = "1.31.0"is incorrect , since its a binary, the correct way to use it is to usecargo install cargo-instaand thencargo insta review