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

[Feature] Migrate validity.rs test to pure Amber files #292

Closed
Mte90 opened this issue Jul 9, 2024 · 5 comments · Fixed by #293
Closed

[Feature] Migrate validity.rs test to pure Amber files #292

Mte90 opened this issue Jul 9, 2024 · 5 comments · Fixed by #293
Assignees
Labels
enhancement New feature or request

Comments

@Mte90
Copy link
Member

Mte90 commented Jul 9, 2024

Like as we did for stdlib also that file need that "cure".

After the merge of #291 we can work also on this one.

@Mte90 Mte90 added the enhancement New feature or request label Jul 9, 2024
@Mte90 Mte90 self-assigned this Jul 9, 2024
@Ph0enixKM
Copy link
Member

Ph0enixKM commented Jul 9, 2024

@Mte90 I disagree. The validity tests are for compiler developers. They already know how rust works and probably expect something that is more idiomatic to Rust projects.

I also don't see the reason to do that - also because testing stdlib now takes forever (like 2-5 minutes) when we have separated the files out. And the validity completes in 5 seconds

@b1ek
Copy link
Member

b1ek commented Jul 10, 2024

also because testing stdlib now takes forever

its a synchronous testing issue. cargo test by default splits them out to multiple threads, and we can split them out into different threads with little to no effort

@Ph0enixKM
Copy link
Member

also because testing stdlib now takes forever

its a synchronous testing issue. cargo test by default splits them out to multiple threads, and we can split them out into different threads with little to no effort

We can do that. But I still don’t understand why should de separate the validity tests

@Mte90
Copy link
Member Author

Mte90 commented Jul 10, 2024

Because we are talking about a new language, with tests inside a Rust files it is required to escape everything, the readability is bad.
Also when the editor will support Amber language with various files it will be more easier to manage them, also it is more faster to add new tests with creating files.

PHP, PYthon and other languages doesn't include this tests in C files as example https://github.com/php/php-src/tree/master/tests/func

@Ph0enixKM
Copy link
Member

Okay then. But the PR should also improve the performance of the tests as well

@Mte90 Mte90 linked a pull request Jul 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants