Skip to content
This repository was archived by the owner on Apr 8, 2022. It is now read-only.

script: Add some exhaustive testing #124

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

Conversation

iljakuklic
Copy link
Contributor

No description provided.

@iljakuklic iljakuklic added the testing Issues related to testing label Nov 15, 2021
@iljakuklic iljakuklic requested review from zorvan and altonen November 15, 2021 11:31

// Test the interpreter on all 4-byte combinations of non-trivial opcodes.
#[test]
//#[ignore = "This test is too expensive to run by default"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to comment the ignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. It runs pretty quickly in release mode, but under debug, I will have to measure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running this test with debug build takes 147 seconds on my laptop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do we aim for in these tests? Are tests that run a long-ish time under debug acceptable or do we want to disable them by default (make opt-in)?

Copy link
Contributor

Choose a reason for hiding this comment

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

147s is okay for me but is probably approaching the limit of "I'm getting bored waiting for this"-ness so maybe making them opt-in is a good idea. If someone touches the script stuff then they know to run them anyway so we're probably safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In release mode, it is about 5s on my laptop btw

@iljakuklic iljakuklic force-pushed the script-exhaustive-tests branch from f7f742e to e6aca48 Compare November 15, 2021 13:55
@muursh muursh self-requested a review November 17, 2021 10:28
@iljakuklic iljakuklic force-pushed the script-exhaustive-tests branch from e6aca48 to 34f9b90 Compare November 17, 2021 11:34
@iljakuklic iljakuklic force-pushed the script-exhaustive-tests branch from 34f9b90 to 53a235e Compare November 18, 2021 12:45
@iljakuklic iljakuklic force-pushed the script-exhaustive-tests branch from 53a235e to 600ea45 Compare November 29, 2021 13:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
testing Issues related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants