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

tests: add tests for injecting large resources #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

RaisinTen
Copy link
Contributor

@RaisinTen RaisinTen commented Sep 9, 2022

Fixes: #12
Signed-off-by: Darshan Sen raisinten@gmail.com

@RaisinTen RaisinTen force-pushed the add-tests branch 2 times, most recently from bacb589 to f5acc1d Compare September 20, 2022 06:39
@RaisinTen RaisinTen changed the title Add tests Add tests for injecting large resources Sep 20, 2022
@RaisinTen RaisinTen changed the title Add tests for injecting large resources tests: add tests for injecting large resources Sep 20, 2022
@RaisinTen RaisinTen force-pushed the add-tests branch 2 times, most recently from 581d0ce to cbfc064 Compare September 20, 2022 11:29
Fixes: #12
Signed-off-by: Darshan Sen <raisinten@gmail.com>
build/deps.mk Outdated Show resolved Hide resolved
cc test.c -o "$bin"

input="$TEMPORARY_DIRECTORY/input.txt"
head -c 1073741824 /dev/urandom > "$input"
Copy link
Member

Choose a reason for hiding this comment

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

why not head -c 1G /dev/urandom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@robertgzr doesn't work on macOS unfortunately

Copy link
Member

Choose a reason for hiding this comment

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

sad

RaisinTen added a commit that referenced this pull request Sep 20, 2022
Addresses #7 (comment).

Signed-off-by: Darshan Sen <raisinten@gmail.com>
RaisinTen added a commit that referenced this pull request Sep 20, 2022
Addresses #7 (comment).

Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen RaisinTen requested a review from jviotti September 21, 2022 05:17
Copy link
Contributor

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

I think we should hold off on landing changes to tests until #8 lands, it changes how tests are implemented.

@RaisinTen
Copy link
Contributor Author

it changes how tests are implemented.

Can we focus that PR on just the WASM-related changes instead? I don't think it's a good idea to mix unrelated things into a PR if we can help it because as we can see here, it's blocking the progress of other PRs. Feel free to let me know if you're stuck anywhere because I've noticed that the last push was 11 days ago.

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 a test injecting a large resource
3 participants