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

Add workflow file to run arm-based testing with sanitizers #649

Merged
merged 7 commits into from
May 10, 2024

Conversation

CarlosEduR
Copy link
Member

Fixes: #520

@lemire
Copy link
Member

lemire commented May 4, 2024

Running tests.

@CarlosEduR
Copy link
Member Author

Seems it failed... I just pushed an update, let's see how it goes.

@lemire
Copy link
Member

lemire commented May 4, 2024

Running...

@CarlosEduR
Copy link
Member Author

@lemire hmm, it is still failing... Off the top of your head, do you have any idea?
image

- 'docs/**'

permissions:
contents: read
Copy link
Member

Choose a reason for hiding this comment

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

So you specified that files could not be written, correct?

Copy link
Member

Choose a reason for hiding this comment

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

I would just drop this permission step and see whether the permission issue resolved itself.

I am not quite sure why we specify permissions... :-/

Copy link
Member Author

@CarlosEduR CarlosEduR May 8, 2024

Choose a reason for hiding this comment

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

Yes, actually I was more trying to follow the pattern I saw in other files. Also, the workflow file you shared with me as an example, it actually does not contain any permissions. I was wondering if adding permissions to write could be an issue.

cmake -DADA_SANITIZE=ON -DADA_DEVELOPMENT_CHECKS=ON -DBUILD_SHARED_LIBS=${{matrix.shared}} -G Ninja -B build
cmake --build build -j=2
- name: Test
run: ctest --output-on-failure --test-dir build
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we could try removing --output-on-failure which might prevent ctest from creating logs, but I am not sure.

@lemire
Copy link
Member

lemire commented May 8, 2024

It looks like a permission problem. I am not sure what the source of the issue is.

@lemire
Copy link
Member

lemire commented May 8, 2024

I have never seen this error:

CMake Error: Cannot open file for write: /home/runner/work/ada/ada/build/Testing/Temporary/LastTest.log.tmp
CMake Error: : System Error: Permission denied

@lemire
Copy link
Member

lemire commented May 9, 2024

Let us run the tests...

cmake -DADA_SANITIZE=ON -DADA_DEVELOPMENT_CHECKS=ON -DBUILD_SHARED_LIBS=${{matrix.shared}} -G Ninja -B build
cmake --build build -j=2
- name: Test
run: ctest --test-dir build
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be inside the previous step just like the s360x workflow.

Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

Can you also readd the permissions? It's important to limit the abilities of the workflow packages.

.github/workflows/aarch64.yml Outdated Show resolved Hide resolved
@lemire
Copy link
Member

lemire commented May 10, 2024

@anonrig I suspect that the memory sanitizer is still busted. :-(

It finds problems inside the standard library.

(It is clearly unrelated to this PR.)

@anonrig anonrig merged commit ccb7a26 into ada-url:main May 10, 2024
36 of 37 checks passed
@CarlosEduR
Copy link
Member Author

hmm, I see the pipeline passed, but what I saw from logs:
image

@lemire
Copy link
Member

lemire commented May 10, 2024

@CarlosEduR Yes. But at least we are now checking that things build, which is good.

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

Successfully merging this pull request may close these issues.

Add ARM-based testing with sanitizers in continuous integration
3 participants