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

Test output as comment #335

Merged
merged 11 commits into from
Jul 26, 2024
Merged

Test output as comment #335

merged 11 commits into from
Jul 26, 2024

Conversation

Mte90
Copy link
Member

@Mte90 Mte90 commented Jul 23, 2024

Based on #332 this is my proposal with some tests as example and works with multiline output.

In this way:

  • If there is a comment in the code that starts with // Output it will get that content for the test check
  • Otherwise
    • If there is a *.output.txt file will read from that
      • Otherwise will use Succeded as string for the check

I think that in this case we can simplify a lot and cover all the needs for the future.

If I get an approval I will move on to migrate (again) all the tests (also for validity) with the contribute guide.

Copy link
Member

@Ph0enixKM Ph0enixKM left a comment

Choose a reason for hiding this comment

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

Some small changes. I'll think more about the actual implementation. I thought that we count only the comments at the top to be the output and not the comments that start with "Output". Give me some time to let that sink in

src/tests/stdlib.rs Outdated Show resolved Hide resolved
src/tests/stdlib.rs Show resolved Hide resolved
Co-authored-by: Phoenix Himself <[email protected]>
@Mte90
Copy link
Member Author

Mte90 commented Jul 23, 2024

About Output on top, it is just to give a bit of context in case someone else open those files and doesn't understand this comments.
Also in case it is needed to add some documentation as some tests has already.

Copy link
Member

@b1ek b1ek left a comment

Choose a reason for hiding this comment

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

this gotta be explained in CONTRIBUTING.md tho

@Mte90
Copy link
Member Author

Mte90 commented Jul 26, 2024

this gotta be explained in CONTRIBUTING.md tho

Yes I will do that when we define how the comment is structured.

@Mte90 Mte90 requested review from b1ek and Ph0enixKM July 26, 2024 12:13
Copy link
Member

@Ph0enixKM Ph0enixKM left a comment

Choose a reason for hiding this comment

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

Looking good

src/tests/stdlib.rs Outdated Show resolved Hide resolved
Copy link
Member

@b1ek b1ek left a comment

Choose a reason for hiding this comment

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

just like i said, it gotta be explained in CONTRIBUTING.md

@Mte90 Mte90 requested a review from b1ek July 26, 2024 13:34
@b1ek b1ek merged commit 038a5b3 into amber-lang:master Jul 26, 2024
1 check passed
@Mte90 Mte90 deleted the test-comment-output branch September 5, 2024 13:03
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.

3 participants