-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add tests to CI for Windows (and maybe others) #117
Comments
@swinslow , I can try to handle this issue! |
That's great -- thank you @CatalinStratu! |
Thanks, I'll take care of this task after I'm done with task #120 |
Thanks @CatalinStratu. If there are some tests that need refactoring, that makes sense and that's fine to include in a proposed PR. I'd suggest breaking up refactoring changes into small separate PRs wherever possible so that they can be discussed one by one, rather than one very large PR which would be harder to evaluate. On the specific failures from your screenshot, note that the verification code and number of Files should be the same regardless of whether the Golang tools are being run on Windows or on Linux. The fact that they're different here is a good thing; that means that the tests are identifying a problem with the tools-golang that is leading to different results on the different platforms. We should dig into why the results are different, and figure out how to fix the bug in the tools -- rather than changing the tests to hide the bug :) For this particular test, it looks like this is where the Can you submit a separate issue for this particular test error, so that we can dig into that specifically? Resolving that bug is going to take a bit of digging to see how symbolic links are handled for other SPDX tools, and it's a bit separate from this issue #117 for enabling the Windows tests to run within the GitHub Actions CI system. Thank you! |
Oh, I forgot to mention -- in case you're wondering what this verification code thingy is, section 7.9 of the SPDX spec has more details. |
Hiya, @swinslow and @lumjjb -- a user submitted a PR with a fix for Windows, so I thought I'd look into getting the tests running on Windows. As expected, there are some path separator (and other) issues, but I'm not entirely sure the "right" thing to do here. I don't use any of this functionality, so I really don't want to make changes that would break things for people, but I see a couple of issues:
To me, I would expect scanning the same set of files on either file system would result in the exact same packages, which tells me that we should be normalizing these to Unix paths. This would also lean into supporting unix paths for exclude patterns on both OSes (and possibly also allowing Windows-style exclude paths). But again, I've not used any of these functions, and would like some guidance what people think is the right thing to do here. Update: SPDX 2.3 spec references the URI spec as the file name, which indicates to me these all should, in fact be normalized to URI-like paths |
Since Windows handles filepaths differently (and probably some other things I'm not thinking of), we should also configure the GitHub Actions testing to run on a Windows VM -- if that's doable within GitHub Actions. Should also look into whether to do the same for e.g. MacOS and/or others.
The text was updated successfully, but these errors were encountered: