Refactoring javascript code and introduce js testing#154
Refactoring javascript code and introduce js testing#154mwarning merged 2 commits intoopenwrt:mainfrom
Conversation
|
hi @andibraeu , but you should remove the yarn.lock file. |
|
I added this file by intention. Having that lockfile leads to using exact the same versions of dependencies. So we should have always the same results. That's good practice in software development projects. |
|
@mwarning can you please approve the workflow to run? |
|
Overall this looks good. But I fear that I will have trouble to keep the tests up to date and will remove tests when I have to fight them too much. :P |
|
@aparcar any opinions from your side? |
|
I'm not a JS dev so can't really judge. Modularity is always a plus for me. If @andibraeu joins the fun to maintain this, excellent! Large commits with lots of tests strike me as a massive AI undertake, which I don't find problematic but it may decrease maintainability in the long term. Not sure... |
|
I want to help improve quality of this software. So I splitted up this huge js file (as a preparation for some other changes I want to propose), and added unit tests to see if changes in code change other behaviour than expected. That's my standard for modern software ;) and IMO tests increase maintainability. (and yes, test cases were created using AI) For the tests I chosed an easy way, so no big framework is needed, just nodejs. |
|
Ok, then let's merge it once the bug has been fixed and the commit messages have been cleaned up. And @andibraeu will maintain the tests. :D |
796b571 to
51ef7b1
Compare
adds unit tests for js part runs unit tests in ci updates ci actions Signed-off-by: Andreas Bräu <ab@andi95.de>
|
@andibraeu @aparcar I would like to mege now. But the pipeline complains that it cannot do the Setup Pages step? I do not know what it is meant to do. We do not push static pages. Can someone help me out? |
|
I only see that in my forked repo, but there I did not setup github pages. I enabled pages by now, and the job worked |
|
BTW @mwarning : there's a github action that deploys to github pages: https://openwrt.github.io/firmware-selector-openwrt-org/ There are 2 workflows doing that: https://github.com/openwrt/firmware-selector-openwrt-org/tree/main/.github/workflows |
|
@andibraeu I see. :) I re-approved the workflow, let's see if it succeeds. |
this PR does the following: