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

Massive Refactor for jest testing #1176

Closed
wants to merge 6 commits into from
Closed

Conversation

O-Mutt
Copy link
Contributor

@O-Mutt O-Mutt commented Oct 22, 2019

This modifies all the tests to run under jest. There are a lot of broken tests still.

Ref:
#1136 #1137

Copy link
Collaborator

@kirjs kirjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥First pass for the obvious things, generally looks good

coverageDirectory: '../../coverage/libs/utils'
};

/*module.exports = function(config) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's much value in keeping the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was leaving those for when I was testing to make sure if there were imports/anything that i could use in the jest.config. I will clear them for sure


/*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop the config?

@@ -1,20 +1,25 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { FullScreenRunnerComponent } from './full-screen-runner.component';
import {
CUSTOM_ELEMENTS_SCHEMA,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of those imports not used

@@ -1,20 +1,25 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { BindecComponent } from './bindec.component';
import {
CUSTOM_ELEMENTS_SCHEMA,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CUSTOM_ELEMENTS_SCHEMA - is this used?

@@ -1,20 +1,25 @@
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { BinaryPlainComponent } from './binary-plain.component';
import {
CUSTOM_ELEMENTS_SCHEMA,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

@@ -2,7 +2,7 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { DebuggerComponent } from './debugger.component';

describe('DebuggerComponent', () => {
describe.only('DebuggerComponent', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop .only

@kirjs
Copy link
Collaborator

kirjs commented Oct 22, 2019

wow, this looks awesome!

Curious if you were in touch with @hansal7014 (from #1136) to avoid duplicate work?

@O-Mutt
Copy link
Contributor Author

O-Mutt commented Oct 22, 2019

wow, this looks awesome!

Curious if you were in touch with @hansal7014 (from #1136) to avoid duplicate work?

Did not, i must have started before those comments came in. It took me a few days of messing around to get everything working well enough

@kirjs kirjs mentioned this pull request Oct 22, 2019
@kirjs
Copy link
Collaborator

kirjs commented Oct 22, 2019

Ok, cool, good to know, I pinged the other thread to avoid doing the same stuff twice in the future, for now let's merge this in

@O-Mutt
Copy link
Contributor Author

O-Mutt commented Oct 22, 2019

Ok, cool, good to know, I pinged the other thread to avoid doing the same stuff twice in the future, for now let's merge this in

I'm pulled the travis changes because there are still a ton of broken tests.

@NothingEverHappens
Copy link
Collaborator

Just wanted to send a friendly ping to understand the state of this and make sure it's not blocked from our side.

@O-Mutt
Copy link
Contributor Author

O-Mutt commented Nov 4, 2019

Hey @NothingEverHappens this was not blocked just haven't had a time to get back to the enormity of this. This in and of itself should be ok if we wanted to take a small chunk out of the project

@NothingEverHappens
Copy link
Collaborator

Cool, would you have some time to address the feedback? (I think it's mostly dropping unused code bytes, and other small things)?

I think this is a big step forward compered to what we have now and would be awesome to get merged.

@NothingEverHappens
Copy link
Collaborator

Hey @Mutmatt, are you planning to look into merging this in?
Or alternatively I can work on doing this.

@O-Mutt O-Mutt closed this May 4, 2023
@O-Mutt O-Mutt deleted the 1136 branch May 4, 2023 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants