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

Add a test and enable GitHub Actions (CI) #62

Merged
merged 5 commits into from
May 3, 2023
Merged

Add a test and enable GitHub Actions (CI) #62

merged 5 commits into from
May 3, 2023

Conversation

szabgab
Copy link
Contributor

@szabgab szabgab commented Apr 25, 2023

The test uses the default example and the default configuration and compares the results to an earlier set of output.
There is a lot to improve on the tests.

Was created during a session of OSDC

Copy link

@yewtc yewtc left a comment

Choose a reason for hiding this comment

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

I'm also on the OSDC course and am trying out reviewing the change.
Clearly there could be much more, but at least it's a start. We could extend to use different OS's and perl versions.

@lordfeck
Copy link
Owner

lordfeck commented Apr 28, 2023

thanks for the contribution, a bit of CI to automate testing would be helpful! I'll look at getting some of this integrated, will see if we can get this merged with a new branch on my repo

@lordfeck lordfeck changed the base branch from master to add_ci April 28, 2023 09:26
@szabgab
Copy link
Contributor Author

szabgab commented Apr 28, 2023

Why on a branch? The idea of CI is to run on the code as you develop it.

@lordfeck lordfeck changed the base branch from add_ci to master May 3, 2023 20:23
@lordfeck
Copy link
Owner

lordfeck commented May 3, 2023

@szabgab Fair point. I was thinking I could improve it by dumping it to a temp directory using ./rsru.pl -o ./tmp and then parsing the HTML and XML output using some CPAN modules. That would make the test fully deterministic.

But this gives me a good start so I'll merge it to master now.

@lordfeck lordfeck merged commit f06e79d into lordfeck:master May 3, 2023
@szabgab
Copy link
Contributor Author

szabgab commented May 4, 2023

Thanks for accepting.

In general you'd want to make the tests as simple as possible so they won't add extra complexity.
I'd probably create a few examples in the source code with expected output saved also in git and would make them without a timestamp so I won't need to exclude anythings.

One would also need to fix the test environment to make sure the images are generated the same way and exclude the images in cases where they might be different due to different versions of libgd.

@szabgab szabgab deleted the ci branch May 4, 2023 03:58
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.

None yet

3 participants