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

Cleaned up stress tests. #829

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

supercooldave
Copy link

Added entries into IGNORED_FILES.grep to reduce number of warning produced when running stress tests.

Made one Makefile a little more robust.

@albertnetymk
Copy link
Contributor

Listing each directory explicitly under stress seems an unnecessary burden to maintain, for we know that they are stress tests, not ordinary tests. (Doesn't the ignore scheme in the testing script support whole directory exclusion?)

Using explicit relative path while invoking the compiler seems a bit odd as well. ../../../../../release/encorec -O3 main.enc

@supercooldave
Copy link
Author

If you don't mention the directories, then the screen is filled with warnings, though the tests will still pass.
It was decided not to suppress such warnings and to force you, the Encore developer, to explicitly add entries to IGNORED_FILES.grep, to avoid accidentally submitting a test file (.enc) without the corresponding other files.

I added the explicit path to the encore compiler to make sure that it was running the right one — perhaps this is redundant and handled already by the testing script.

Copy link
Contributor

@kikofernandez kikofernandez left a comment

Choose a reason for hiding this comment

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

Looks good but I won't merge until the author confirms if a Makefile is needed to do the stress testing. If the author fixes this issue before 15.00, I'll merge today


all: main

run: main
/usr/bin/time ./main

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this Makefile needed by the CI?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, because there is no other way to pass the -O3 flag to the compiler.
(Whether the -O3 flag is really required is a question for the BoundedBuffer implementor.)
The reason for tidying up the Makefile was to reduce errors in the output, and increase faith in make stress.

@supercooldave supercooldave force-pushed the stressful-errors branch 3 times, most recently from 77ca53d to c7e22e3 Compare July 6, 2017 17:36
@supercooldave
Copy link
Author

@kikofernandez Ping!

@supercooldave
Copy link
Author

@kikofernandez Are you fishing for more cake?

@kikofernandez
Copy link
Contributor

CI fails

Sent from my Samsung GT-I9506 using FastHub

@supercooldave
Copy link
Author

CI fails because I keep updating src/tests/IGNORED_FILES.grep whenever I add new (stress) tests.
I'll fix soon and checkout that Makefile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants