-
Notifications
You must be signed in to change notification settings - Fork 38
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 tests #228
Comments
And probably explicit the need to add test to new functionality-related PR? |
I haven’t looked at static’s tests yet but perhaps we should first figure out what needs to be tested? Do you envision testing the output of tasks or the internals that make up the tasks themselves or both? I agree with the importance and value of adding tests, they will make refactoring and updating a lot quicker. |
On Thu, 28 Feb 2019 at 03:33, Jay ***@***.***> wrote:
Do you envision testing the output of tasks or the internals that make up
the tasks themselves or both?
Given the current state of affairs, I would proceed in exactly that order
:-)
This would result in an "outside-in" approach: to tackle one output task at
a time, and only then the internals, one by one.
Following this logic, `markdown` and `markdown*` would be among the first
tasks to work on.
… |
Sounds like a good strategy to me. Next, how do we test these output tasks? Can we run these tasks against fixture markdown files and take snapshots of the output then compare them to catch regressions? In my mind we want to make sure the markdown tasks transform the markdown expectedly but without having to write too many tests. What are your thoughts? |
On Thu, 28 Feb 2019 at 18:16, Jay ***@***.***> wrote:
Sounds like a good strategy to me.
:-)
Next, how do we test these output tasks? Can we run these tasks against
fixture markdown files and take snapshots of the output then compare them
to catch regressions?
Something like that. I would strongly advise you to check static's tests
for more concrete details :-)
(I had no experience in this kind of testing myself before coming across
static's code.)
… |
I'll definitely read through it 🙂 but I want to make sure we're on the same page about what we should test and how we should test it first. I've shot myself in the foot looking at other implementations too early and trying to make my problems fit into them instead of understanding my goals and expected outcomes ahead of time. |
After taking a look it inspired a good deal of thoughts: This feels ok but I think using Clojure to write the dummy files is an extra maintenance step. I would prefer if we had regular md, images, css, etc... files in a folder we can use directly as it will be easier to maintain and closer to the real user experience. Additionally, since boot is immutable we don't have to worry about copying the files. We would just need to clean the target folder (which I already have a recursive function for) or more easy: ignore the target dir using gitignore. Then create another folder that is the expected text output of the transformations. Then in our tests we can split them into seqs of lines and diff them using clojure.test tools so we can pinpoint what lines are different between transformations. I'm happy to get the plumbing started if it would be of help to you. |
On Thu, 28 Feb 2019 at 19:17, Jay ***@***.***> wrote:
After taking a look t inspired a good deal of thoughts [...]
Thank you for your comments.
I'm happy to get the plumbing started if it would be of help to you.
Please, be my guest :-)
I will be quite busy until the end of this week myself. I have found myself
working in two fronts at once: on one side, working on a website; on the
other side, fixing the tool that helps me create the website. And it is
time for me to catch up with the website I am supposed to build :-)
As I stated in the beginning of this thread, this bug was intended to
(just) spark the discussion towards a mid-term goal. I'm happy it has
out-served its purpose :-)
|
Even though this may be a long goal, I think it would be nice to add tests to the source code.
As a starting point, we could use these tests, from static, a project similar to perun.
(BTW: Please note that static's code is licensed under BSD-2. So, anyone using it should acknowledge it properly.)
The text was updated successfully, but these errors were encountered: