Enable GitHub Actions - Githubissues

omasanori commented 1 year ago

This change enables a CI workflow for OpTeX.

It builds OpTeX with the latest TeX Live and typeset demo files on every pull request. You can download the typeset files for three days if succeeded.

I hope this will become the basis of more advanced automatic testing on OpTeX and prevent accidental breakages.

cao- commented 1 year ago

Well, I see that you typeset the demo/test files, so we can spot if a macro fails (because the typesetting job fails), but how can we spot visual changes? Maybe a macro doesn't issue errors but it is not behaving as expected, and produces a different visual result. Would it be possible to have a visual test that visually compares the expected pdf with the resulting pdf? Then if the change is actually expected, one can simply approve the change, otherwise the unexpected change is spotted. If possible, I guess this requires to keep in the repo the pdf files generated for each test file, so that they can be compared with the ones we get on each push and eventually will be updated.

olsak commented 1 year ago

I am doing much more tests on my local machine (using local script) before I do git push. This script runs OpTeX to more my documents. Of course, the visual checking of resulting PDFs is done only roughly and manually. I am not sure if I want to have a testing script on the github. I use github as an archive of files only. I don't know all features of git/github. If there is a possibility to use such script by people which wants to use it, OK. But I don't plan to use it.

robertbachmann commented 1 year ago

If there is a possibility to use such script by people which wants to use it, OK. But I don't plan to use it.

GitHub will run the workflow from the YAML file everytime someone creates a pull request. I think it's useful as quick "smoke test" for pull requests.

Example: John Doe creates a pull request, GitHub will then run the "smoke tests". It will display the result in the Pull Request interface.

If the test fails John Doe immediately knows that he needs to fix his code, and you know that you don't need to review/test it it - since it's obviously broken
If the test passes, then you can review the code and do your enhanced tests on your local machine.

omasanori commented 1 year ago

Thanks all for your responses.

As @robertbachmann pointed out, it is not something making manual tests @olsak does unnecessary. Rather, it complements developers' workflow by signaling No Go against obviously broken changes automatically.

Regarding visual tests @cao- suggested, yes, visual diff against the previous succeeded commit is an natural extension of this PR. However, as in fact I have not yet implemented it and proposing the both CI setup and visual diff altogether makes review harder, it is not included in this PR.

olsak commented 1 year ago

I don't want to run any tests on github server side when I do git push because I provide more tests on my local box before git push. But if somebody else want it then he/she can include such YAML script. My question is if it is possible to have such YAML script only at forks of my master (no my master itself), it means (for examle) at https://github.com/robertbachmann/OpTeX or https://github.com/omasanori/OpTeX.

omasanori commented 1 year ago

While it is possible, I would propose that the tests will run on pull requests but not on push to master and scripts themselves are in the canonical repository.

omasanori commented 1 year ago

I made the CI triggered only on pull requests.

olsak commented 1 year ago

I have no knowledge about it and I'm worried. I'm not able to imagine at what situations the script is started. If it is started only if a user want it (i.e. after a bottom click) then there is no problem. But if it is more automatic and the script will run quite a long time then it will be frustrating. For example, I'm waiting for the message "this PR has no conflict..." about one second now but I don't want to wait twenty seconds.

cao- commented 1 year ago

I'm not sure, but I guess that it is automatic and that you can still immediately see if the PR has merge conflicts or not, and only later after the script has run you can also see if the code in the PR has passed the tests. Please correct me if I'm wrong

omasanori commented 1 year ago

It runs automatically when a pull request is created and any new commit is pushed to that pull request, and the result (succeeded or failed) is kept until a new commit is pushed to the pull request.

So, unlike merge conflict check, it will not be triggered everytime you check the page, nor it will not rebuild everything whenever you pushed a commit to your master branch.

I have tested this script on my fork and it takes few minutes when triggered. However, unless you complete the local checks within a minute or such immediately after you are notified a pull request, it will not let you wait that much.

omasanori commented 1 year ago

Also, unless you have configured branch protection in the settings tab, the CI will not lock your merge button. It is opt-in.

vlasakm commented 1 year ago

Just some ideas:

IMHO it would be more useful to start with building something that the CI can run and thinking about what we want to test.

"Smoke testing" demo files by trying if they compile is a solid start, but won't uncover many real issues.

I think the following would be worthwhile testing:

Past issues - there could be a test file for each past issue, so we can make sure we don't regress the issue again.
OpTeX tricks - with changes to OpTeX they can get subtly out of date, we should ensure they all still work.
Tests against other packages and their various versions. OpTeX is not free of dependencies. It even depends on some details of luatex, luaotfload and hyph-utf8. We may want to support a range of versions of these packages (for example if a user installs latest OpTeX with their TeX Live 2020 or whatever). We might also want to test against development versions of these packages so that we can prevent future breakage.
Same thing applies to optional packages OpTeX can use: tikz, minim, luavlna, emoji, pdfextra, lua-widow-control.

Also, instead of visual testing by e.g. converting PDFs to images and comparing the images, testing the output of \tracingoutput=1 could be realistic. AFAIK this is what l3build does and it might be easier to leverage it. See for example https://github.com/latex3/luaotfload/tree/main/testfiles.

E.g. the following:

\fontfam[lm]
\load[tikz]

\tracingoutput=1

{\Red a}b

\tikzpicture [remember picture,overlay]
\draw [line width=1mm,opacity=.25] (current page.center) circle (3cm);
\endtikzpicture

\bye

Produces:

\vbox(718.24724+0.0)x455.24408, direction TLT
.\vbox(694.24724+0.0)x455.24408, glue set 672.24725fill, direction TLT
..\glue(\topskip) 3.06
..\hbox(6.94+0.11)x455.24408, glue set 424.68408fil, direction TLT
...\localpar
....\localinterlinepenalty=0
....\localbrokenpenalty=0
....\localleftbox=null
....\localrightbox=null
...\hbox(0.0+0.0)x20.0, direction TLT
...\pdfliteral direct <lua data reference 785>
...\_ten_tenrm a
...\pdfliteral direct <lua data reference 797>
...\_ten_tenrm b
...\penalty 10000
...\glue(\parfillskip) 0.0 plus 1.0fil
...\glue(\rightskip) 0.0
..\glue(\parskip) 0.0 plus 1.0
..\glue(\baselineskip) 11.89
..\hbox(0.0+0.0)x455.24408, glue set 435.24408fil, direction TLT
...\localpar
....\localinterlinepenalty=0
....\localbrokenpenalty=0
....\localleftbox=null
....\localrightbox=null
...\hbox(0.0+0.0)x20.0, direction TLT
...\hbox(0.0+0.0)x0.0, direction TLT
....\glue 0.0
....\hbox(0.0+0.0)x0.0, direction TLT
.....\pdfliteral direct <lua data reference 798>
.....\pdfliteral origin{q }
.....\pdfliteral origin{0 0 0 RG }
.....\pdfliteral origin{0 0 0 rg }
.....\pdfliteral origin{0.3985 w }
.....\hbox(0.0+0.0)x0.0, direction TLT
......\pdfliteral origin{q }
......\glue(\spaceskip) 0.0
......\glue(\spaceskip) 0.0
......\pdfliteral origin{q }
......\pdfliteral origin{2.83466 w }
......\pdfliteral origin{/pgf@CA.25 gs }
......\pdfliteral origin{/pgf@ca.25 gs }
......\pdfliteral origin{206.84904 -328.16516 m }
......\pdfliteral origin{291.8895 -328.16516 m }
......\pdfliteral origin{291.8895 -281.19801 253.8162 -243.1247 206.84904 -243.1247 c }
......\pdfliteral origin{159.8819 -243.1247 121.80858 -281.19801 121.80858 -328.16516 c }
......\pdfliteral origin{121.80858 -375.13231 159.8819 -413.20563 206.84904 -413.20563 c }
......\pdfliteral origin{253.8162 -413.20563 291.8895 -375.13231 291.8895 -328.16516 c }
......\pdfliteral origin{h }
......\pdfliteral origin{206.84904 -328.16516 m }
......\pdfliteral origin{S }
......\glue(\spaceskip) 0.0
......\pdfliteral origin{Q }
......\glue(\spaceskip) 0.0
......\pdfliteral origin{Q }
......\hbox(0.0+0.0)x0.0, direction TLT
.......\savepos
.......\write1{\_bslash \_csstring \_pgf_Xpgfsysmark {pgfid1}{\_the \_lastxpos }{\_the \_lastypos }}
......\glue 0.0 plus 1.0fil minus 1.0fil
.....\pdfliteral origin{n }
.....\pdfliteral origin{Q }
.....\glue 0.0 plus 1.0fil minus 1.0fil
...\penalty 10000
...\glue(\parfillskip) 0.0 plus 1.0fil
...\glue(\rightskip) 0.0
..\glue 0.0 plus 1.0fill
..\kern0.0
..\glue 0.0
.\glue(\baselineskip) 17.34
.\hbox(6.66+0.0)x455.24408, glue set 225.12204fil, direction TLT
..\glue 0.0 plus 1.0fil minus 1.0fil
..\_ten_tenrm 1
..\glue 0.0 plus 1.0fil minus 1.0fil

An alternative might be something like the nodetree package, which formats the node lists in a custom way.

omasanori commented 1 year ago

Thank you for your suggestions, @vlasakm !

Yes, it is better to have some practical tests. Moreover, I think that it would be excellent if anyone can run make check before submitting a PR. I will do the integration if the Makefile proposal lands.

Also, instead of visual testing by e.g. converting PDFs to images and comparing the images, testing the output of \tracingoutput=1 could be realistic.

I did not come up with it, marvellous!

olsak commented 1 year ago

I will do the integration if the Makefile proposal lands.

The airport tower reports: Makefile landed now.

robertbachmann commented 1 year ago

@omasanori. AFAIK we can replace

      - name: Deploy
        run: |
          mkdir -p $HOME/texmf/web2c/luatex
          mkdir -p $HOME/texmf/tex/optex
          cp -r optex/base optex/pkg $HOME/texmf/tex/optex
      - name: Build and install
        run: |
          luatex --credits
          luatex -ini optex/base/optex.ini
          mkdir -p $HOME/texmf/web2c/luatex
          mv optex.* $HOME/texmf/web2c/luatex
      - name: Build demos
        run: |
          optex optex/demo/op-demo.tex
          optex optex/demo/op-demo.tex
          optex optex/demo/op-slides.tex
          optex optex/demo/op-letter-en.tex
          optex optex/demo/op-letter-cs.tex
          optex optex/demo/op-mathalign.tex

with:

      - name: Build and test
        run: |
          luatex --credits
          cd optex && make format test

olsak commented 1 year ago

Doing diffs on .log files is nothing new. D. Knuth created torture test TRIP in 1990, see https://texdoc.org/serve/tripman.pdf/0. But this was somewhat different situation: the TeX was frozen, the log file format and TeX font metrics of Computer modern were frozen too. Now, luatex and its log file format is still under construction, luaotfload package too, metric part of used Opentype fonts can be changed too. So, I never thought about it. I am not sure that this will be usable and the benefits outweigh the amount of work involved.

omasanori commented 1 year ago

Closing for now. Let's revisit after some automatic tests lands.

olsak / OpTeX

Enable GitHub Actions #116