Skip to content

[SYCL] Disable the LIT test 'debug-info-file-checksum.cpp' for Windows. #6938

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 3, 2022
Merged

[SYCL] Disable the LIT test 'debug-info-file-checksum.cpp' for Windows. #6938

merged 1 commit into from
Oct 3, 2022

Conversation

zahiraam
Copy link
Contributor

@zahiraam zahiraam commented Oct 3, 2022

The LIT test is using append-file. This command seems to have some escape character issue on Windows.
Talked off-line to @pvchupin and agreed that for now the test case can be disabled for windows.

Signed-off-by: Zahira Ammarguellat zahira.ammarguellat@intel.com

@zahiraam
[SYCL] Disable the LIT test 'debug-info-file-checksum.cpp' for Window…
9a3d17c 
…s. The LIT

test is using append-file. This command seems to have some escape character issue
on Windows.

Signed-off-by: Zahira Ammarguellat <zahira.ammarguellat@intel.com>
@zahiraam zahiraam marked this pull request as ready for review October 3, 2022 17:27
@zahiraam zahiraam requested a review from a team as a code owner October 3, 2022 17:27
@pvchupin pvchupin merged commit 40872e5 into intel:sycl Oct 3, 2022
@pvchupin pvchupin mentioned this pull request Oct 3, 2022
Merged
@Fznamznon
Copy link
Contributor

@zahiraam , @pvchupin , could you please clarify why the test was disabled?

The test is added by #6774 which is a windows-specific patch. The test fails on Windows. Doesn't it mean that the patch that added the test is not fully viable or not fully tested now?

@zahiraam
Copy link
Contributor Author

zahiraam commented Oct 5, 2022 •
edited
Loading

@zahiraam , @pvchupin , could you please clarify why the test was disabled?

The test is added by #6774 which is a windows-specific patch. The test fails on Windows. Doesn't it mean that the patch that added the test is not fully viable or not fully tested now?

@Fznamznon no, the feature in this patch to fix debug info on Windows with footer is correct and has been verified and tested by the DGI team for Linux and Windows. The issue is with the test itself on Windows. I am using the command "append-file" (this prepares a temp file for the second compile) in the LIT test ( I don't think it's used anywhere else afaik). This command generates a temp file with a "#line path-to-the-source-code" as a first line. This path on Linux is generated with "/", but on Windows it generates it with "", that seem to be confusing the LiteralString Parser in the call to getPresumedLoc.
I think the fix either to create a temp file by hand but that defeats the purpose of the test, or presumably fix a bug with the append-file tool on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elizabethandrews elizabethandrews elizabethandrews approved these changes

Assignees

@pvchupin pvchupin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zahiraam @Fznamznon @elizabethandrews @pvchupin