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

workflows: fix ci-run-tests.sh script #433

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

perillo
Copy link
Contributor

@perillo perillo commented Apr 3, 2024

The run function is incorrect, since it does not accept additional arguments.

Don't use pwd for sources directory, since all other paths, excluding /tmp, are relative.

Update the ci.yml workflow, removing the "amd64" argument to the ci-run-tests.sh script. It was working because the argument was ignored.

@SimonKagstrom, the change from "pwd" to "." may be controversial. Let me know what you think. I use it in my test.sh script, when testing locally.

The run function is incorrect, since it does not accept additional
arguments.

Don't use `pwd` for sources directory, since all other paths, excluding
/tmp, are relative.

Update the ci.yml workflow, removing the "amd64" argument to the
ci-run-tests.sh script.  It was working because the argument was
ignored.
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.68%. Comparing base (bc0a626) to head (eef552a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #433   +/-   ##
=======================================
  Coverage   65.68%   65.68%           
=======================================
  Files          58       58           
  Lines        4514     4514           
  Branches     4171     4171           
=======================================
  Hits         2965     2965           
  Misses       1549     1549           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SimonKagstrom SimonKagstrom merged commit c5a463d into SimonKagstrom:master Apr 4, 2024
10 checks passed
@SimonKagstrom
Copy link
Owner

I think that's fine, since this is anyway used in a controlled environment.

I personally just use the run-tests script directly, when running the tests.

@perillo
Copy link
Contributor Author

perillo commented Apr 4, 2024

I think that's fine, since this is anyway used in a controlled environment.

I personally just use the run-tests script directly, when running the tests.

I use a custom script: https://gist.github.com/perillo/cebd9f82094471b241b961a4f869b0ed

The script use python -m libkcov, the new way to run the tests (the PR will be pushed soon).

By the way: in the cleanup function I have listed all the generated files; is there a reason why main.log, thread.log and .debug are not written in /tmp ?

In a future PR I plan to move these files to $TMPDIR and later add cleanup functions to remove them. The purpose it enabling support for looping the tests N time (e.g go test -count N); I prefer to not have these files around.

@SimonKagstrom
Copy link
Owner

Not that I can remember, although it was a long time since I initially set this up, so feel free to relocate them as you wish!

@perillo perillo mentioned this pull request Apr 4, 2024
6 tasks
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.

2 participants