ahgilak / deno_gi

Deno port of Gnome libraries (such as Gtk).
https://deno.land/x/deno_gi
33 stars 2 forks source link

Add tests #27

Closed vixalien closed 5 months ago

vixalien commented 10 months ago

Hello! This is a draft PR that adds comprehensive tests. These tests should be run on each PR to avoid regressions by adding a GitHub workflow. This can be done later. Here's the structure I'm suggesting for the tests/ folder:

However, I must admit that I'm not the best at doing test-related stuff, so this may not be the best approach.

Another option to consider is using a more flat directory layout and using periods (.) to structure tests, for example:

As the tests need to be comprehensive, I will do my best to add various tests for each module, and then hopefully, it will get merged. Or we can add the initial tests boilerplate, and add tests as we go. In either case, I am longing for your feedback so I can know how to progress about this.

fixes #14

Unit tests:

ahgilak commented 6 months ago

Hello @vixalien Are you still working on this?

vixalien commented 6 months ago

Are you still working on this?

Yes, very slowly 😭

ahgilak commented 6 months ago

Yes, very slowly 😭

Great that it's still going. there's no rush :) I suggest breaking this into multiple PRs so it would be easier to review and the tests which are ready get merged sooner.

vixalien commented 6 months ago

I suggest breaking this into multiple PRs so it would be easier to review and the tests which are ready get merged sooner.

Alright, I will make something mergeable soon for a few tests, then will add more tests as time passes. Finally we will need to add some integration tests.

One issue is how we will be able to run the "everything" tests. I'm not really that sure how we could build it. I will make a PR once it builds.

vixalien commented 6 months ago

Hi. I just add a simple meson configuration for the everything test, in short it does the following:

  1. downloads the gobject-introspection source
  2. builds libeverything
  3. builds the necessary GIR and typelib
  4. runs the test/everything/everything.ts file with the generated GIR and typelib
  5. we can now use gi://Everything

To run the tests, do the following:

meson setup build
cd build
ninja
# runs the `everything` tests
meson test everything

# you can also directly run the `everything.ts` file using the compiled `everything` library
ninja src/everything/everything.ts

In the future we can add a CI step, and add actual tests.

vixalien commented 6 months ago

We should also add the marshalling tests from https://github.com/GNOME/gobject-introspection/blob/main/tests/gimarshallingtests.c

we will get automatic regression tests for arrays/booleans/everything. With that set up, it will be more easy to experiment since we will be noticing when anything gets broken.

vixalien commented 5 months ago

Update: I've added a couple regress tests in tests/gi/regress adapted from https://gitlab.gnome.org/GNOME/gjs/-/blob/fe6ef0d9f6928521592e3d1558ef5caa5e23817b/installed-tests/js/testRegress.js

This will basically check against a library called Regress to ensure that we match the correct behavior. After this part is done, we will basically have "good" tests, even though I don't think all tests will pass initially.

To run them, just do:

meson setup build
cd build
ninja
# runs the `regress` tests
meson test regress
vixalien commented 5 months ago

Hey, so I think this PR is partially ready. You can review it, and we will continue to implement the other parts from gitlab.gnome.org/GNOME/gjs/-/blob/fe6ef0d9f6928521592e3d1558ef5caa5e23817b/installed-tests/js/testRegress.js in other PRs so that we can iterate faster.

We will need to target the whole GIRegress and GIMarshalling APIs.

ahgilak commented 5 months ago

Are you able to run the examples now?

vixalien commented 5 months ago

Yes i'm able to run the examples. Aren't you able to?

ahgilak commented 5 months ago

No I was getting errors. I think it's fixed now

vixalien commented 5 months ago

Okay, now they are all working (except manual_loop.ts because of a typo)

vixalien commented 5 months ago

I fixed the various pre-existing regressions in #35 (which should be merged first)