ColinEberhardt / assemblyscript-regex

A regex engine for AssemblyScript
MIT License
86 stars 12 forks source link

chore: add as-pect tests and asbuild #19

Closed willemneal closed 3 years ago

willemneal commented 3 years ago

Also fixes virtual method issue I found when testing.

ColinEberhardt commented 3 years ago

Just FYI, the CI failure is due to code formatting:

[warn] Code style issues found in the above file(s). Forgot to run Prettier?

I should probably add this is a commit hook so that it just happens automatically.

ColinEberhardt commented 3 years ago

Just checked out the branch and given this a go - it is much faster :-) 🚀

willemneal commented 3 years ago

@ColinEberhardt I added a git hook and switched to eslint instead of prettier. I've been meaning to make an asfmt package for this purpose, but I used eslint with this configuration in as-pect and near-sdk-as. The rules are adapted from AS's main repo and I added some stuff like unused vars can have _ in front. In general the semantic checks provided by eslint are better than prettier and can handle decorators better. (I found prettier was blowing some of them away).

The tests are also separated, but this slows down as-pect since they were previously all built into one binary. However, it's only two more seconds so not that bad and faster than jest ;-). In the future we will updated as-pect to try to compile all tests into one binary so that you don't have to sacrifice speed for readability.

willemneal commented 3 years ago

Also I still need a way to skip over the tests that aren't working in spec.spec.ts.

MaxGraey commented 3 years ago

I guess integrate with as-pect better delay until AssemblyScript 0.18 published and next as-pect version which will adopted to new runtime as well

willemneal commented 3 years ago

@MaxGraey I would say add as-pect now with lower version and then wait for as-pect to move to v18. I don't think it'll be a quick move for the community to switch to 18. @ColinEberhardt What are you thoughts? Wait for as-pect to catch up to 18?

MaxGraey commented 3 years ago

I afraid current ARC runtime may block progress of development. 0.18 it seems may solve some existing problems.

ColinEberhardt commented 3 years ago

I'm not in any hurry to move to v0.18. It does fix some memory-management related issues (#25), however, other than that, v0.17 works fine for day-to-day development.

I'd be happy to merge this PR and move to as-pect. However, my only concern here is that when AS moves to v0.18 how long it might take for as-pect to upgrade also? In other words, I wouldn't want as-pect to become a limiting factor in the upgrade process.

ColinEberhardt commented 3 years ago

Also I still need a way to skip over the tests that aren't working in spec.spec.ts.

If you look at the current main branch, the spec tests work a little differently now. The specification file is used to generate a more conventional unit test suite:

https://github.com/ColinEberhardt/assemblyscript-regex/blob/main/__tests__/generated/data.js

This allows me to skip tests which I know fail, which means that this can also be added to the CI.

I added a git hook and switched to eslint instead of prettier.

Great, thank you - moving to eslint is a 👍 from me!

Basically, in order to merge this branch I think the only remaining task is to harmonise the approach to spec tests.

willemneal commented 3 years ago

@ColinEberhardt I can adapt your code generator to emit AS too. I was just happy to see includeBytes used but am not attached to including it.

willemneal commented 3 years ago

As for as-pect I'm starting today. It's really really just changing the API's used with rtrace. @jtenner has it down at this point so I'm sure he can push what I do over the finish line.

ColinEberhardt commented 3 years ago

@ColinEberhardt I can adapt your code generator to emit AS too.

That would be great. Once that is done, I would be happy to merge

I was just happy to see includeBytes used but am not attached to including it.

Sorry! looks like a useful library. I'm sure I'll find a use for it in the future.

ColinEberhardt commented 3 years ago

Just FYI I wanted to give this a proper go, so I've finished off the last few steps:

See #27 - if the tests pass, this should be good to go :-)

ColinEberhardt commented 3 years ago

landed via #27 - thanks for your great work @willemneal really appreciated it 👍