Skip to content
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

Framework: Migrate tests from Makefile to single test runner #3942

Closed
gziolo opened this issue Mar 10, 2016 · 11 comments · Fixed by #4294
Closed

Framework: Migrate tests from Makefile to single test runner #3942

gziolo opened this issue Mar 10, 2016 · 11 comments · Fixed by #4294

Comments

@gziolo
Copy link
Member

gziolo commented Mar 10, 2016

Part of #3822.

Makefile list

The following list contains all Makefiles we had when we started migration process:

Please assign yourself to the list when you start working on removing Makefile in favor of single test runner.

√ - it means test suite can be executed independently from other suites using npm run test-client path/to/test/file-name.js
x - it means test suite depends on other suite - we should fix that at some point

@gziolo
Copy link
Member Author

gziolo commented Mar 16, 2016

@mtias can you help us find more devs to speed up migration process? :)

@aduth
Copy link
Contributor

aduth commented Mar 16, 2016

I assigned myself a handful more tests, which I'll plan to start migrating today.

@gziolo
Copy link
Member Author

gziolo commented Mar 16, 2016

@aduth: Awesome, feel free to ping me for a review ;)

I noticed that also @umurkontaci is assigned to some tests, same rule applies of course :D

@umurkontaci
Copy link
Contributor

I have tried client/auth, components/count and components/forms/form-phone-input but they all failed with one way or the other.

@blowery @aduth @gziolo Is there a known way to fix them?

One thing I've encountered was I think we were cleaning up DOM but the components were still trying access window. Some async weirdness.

The other was: this.translate or any this.numberFormat was undefined, I think this was due to this being the wrong context, not just mixins being skipped.

@gziolo
Copy link
Member Author

gziolo commented Mar 18, 2016

@umurkontaci Thanks for working on that. I marked them on the list and I would skip them now, and get back to them once we migrate easier bits :)

dmsnell added a commit that referenced this issue Mar 24, 2016
See #3942

This patch removes the `Makefile` from the `<Interval />` test suite and
adds the tests into the single-test-runner white-list. It updates the
tests to conform to the new specs, using `useFakeDom()` instead of the
older initialization code.

**Testing**

From the project root run `make test`
dmsnell added a commit that referenced this issue Mar 24, 2016
See #3942

This patch removes the Makefile from the ithe importer test suite and
adds the tests into the single-test-runner white-list. No changes to the
tests themselves were needed.

**Testing**

From the project root run make test

cc: @blowery
blowery pushed a commit that referenced this issue Mar 24, 2016
See #3942

This patch removes the Makefile from the ithe importer test suite and
adds the tests into the single-test-runner white-list. No changes to the
tests themselves were needed.

**Testing**

From the project root run make test

cc: @blowery
@stephanethomas stephanethomas self-assigned this Mar 24, 2016
@blowery blowery reopened this Mar 25, 2016
@blowery
Copy link
Contributor

blowery commented Mar 25, 2016

Gotta be careful with those commit messages. 😄

@gziolo
Copy link
Member Author

gziolo commented Apr 20, 2016

@umurkontaci Do you plan to work on removing client/auth/Makefile this week or should someone else take care of it?

@gziolo
Copy link
Member Author

gziolo commented Apr 20, 2016

@umurkontaci Do you plan to work on removing client/auth/Makefile this week or should someone else take care of it?

Forget it, I was able to move it myself, you can help with review :)

@gziolo
Copy link
Member Author

gziolo commented Apr 20, 2016

Done 🎆

@aduth
Copy link
Contributor

aduth commented Apr 20, 2016

Woohoo! 🎉 But to avoid premature celebration, there's still three remaining Makefiles that I believe should be removed:

The first looks to have been migrated already, and should probably be simply deleted. The other two require migration, but appear to be fairly simple.

@gziolo
Copy link
Member Author

gziolo commented Apr 20, 2016

@aduth: 2 of them are removed with #4596 :)
I missed lowercase :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants