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

feat: Testsuite Preview github action #126

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

george-cosma
Copy link
Collaborator

Pull Request Overview

This pull request adds a sticky comment that shows differences between testsuite runs. For example, see george-cosma#11

Based on @nerodesu017 implementation #125

Testing Strategy

This pull request was tested here: george-cosma#11

TODO or Help Wanted

Just as a note, when I tried modifying I32 sub the interpreter ran into an infinite loop. This should be investigated.

Also, this does add a python script. I will want to make another script that tests the speed of our interpreter against a reference program. This script will need to compile another rust program, and storing it in main seems.... weird. Suggestions/Is this a non-issue?

Formatting

  • Ran cargo fmt
  • Ran cargo check
  • Ran cargo build
  • Ran cargo doc
  • Ran nix fmt

@george-cosma
Copy link
Collaborator Author

george-cosma commented Feb 8, 2025

Note, current check fails due to main branch not producing a testsuite_result.json file

w/r/t to the nix workflow, I'll fix it later, until then I am ready to hear your opinions on this

@cemonem
Copy link

cemonem commented Feb 14, 2025

Looks great, I think it's looks fair enough as far as seperation of concerns are concerned at the very beginning, we can change things up later. Once nix deps are added we can merge I think.

@george-cosma
Copy link
Collaborator Author

I'm not sure what you mean by "seperation of concerns" or what "nix dependencies" are referencing

@cemonem
Copy link

cemonem commented Feb 14, 2025

I'm not sure what you mean by "seperation of concerns" or what "nix dependencies" are referencing

By seperation of concerns i mean what the python script does, where in the python script, or rust tests etc. By nix deps I meant the packages that have to be mentioned in the flake as a test dependency / devshell for the python script

@wucke13
Copy link
Collaborator

wucke13 commented Feb 14, 2025

I understand that the python script has no external dependencies, simply putting pkgs.python3 into the devshell could work already?

@cemonem
Copy link

cemonem commented Feb 17, 2025

The contents of the devshell command I added looks bad but fair enough i guess, if there are better ways to write it, be my guest 😅

Copy link

codecov bot commented Feb 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

see 1 file with indirect coverage changes

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