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

Cross Platform Testing TestBuilder2_1CanBuildPackageSection #129

Closed
wants to merge 6 commits into from
Closed

Cross Platform Testing TestBuilder2_1CanBuildPackageSection #129

wants to merge 6 commits into from

Conversation

CatalinStratu
Copy link
Contributor

I propose to do individual tests for Linux and Windows. "/testdata/project1_windows" has no symbolic link because I create it from code. To run the tests on Windows is needed to run the tests as an administrator

Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
Signed-off-by: CatalinStratu <catalinstratu45@gmail.com>
@swinslow
Copy link
Member

swinslow commented Apr 1, 2022

Thanks @CatalinStratu. Before merging this, I'm going to want to do a bit more research into how Git on Windows vs. Linux handles symbolic links.

There's also an open question in the main SPDX specification repo about how symlinks should be handled when creating SPDX documents. Until that issue is resolved, I don't think we'll have a definite answer on how best to handle this.

Instead, I would suggest for the time being let's skip those two particular test cases when the test suite is run on Windows. I'd prefer we skip them, rather than requiring the test suite to run with admin rights (since users rightfully might hesitate to do so).

We'll want to open an issue to track this symlinks question specifically, and then leave it open until the main question in the spdx-spec issue is resolved. In the meantime we can just skip those two failing tests when running on Windows so that we can move forward with setting up the CI for all the other tests. Let me know what you think. Thanks!

@swinslow swinslow added the bug Something isn't working label Apr 1, 2022
@swinslow swinslow added this to the 0.4.0 milestone Apr 1, 2022
@@ -40,7 +40,7 @@ func BuildPackageSection2_1(packageName string, dirRoot string, pathsIgnore []st
for _, fp := range filepaths {
newFilePatch := ""
if osType == "windows" {
newFilePatch = filepath.FromSlash("." + fp[dirRootLen:])
newFilePatch = filepath.FromSlash(fp[dirRootLen:])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"./"

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants