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

[Unit Tests] Update unit tests #256

Merged
merged 9 commits into from
Jan 14, 2025
Merged

[Unit Tests] Update unit tests #256

merged 9 commits into from
Jan 14, 2025

Conversation

jvigliotta
Copy link
Collaborator

@jvigliotta jvigliotta commented Jan 11, 2025

closes #259

Updates unit tests broken during updates/upgrades.

Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I have a couple comments inline.

Did all the other tests pass? I thought for sure the data product cell tests broke as a result of vue upgrade. Did we fix those elsewhere?


import MCWSUserContainerProvider from '../MCWSUserContainerProvider';
import MCWSPersistenceProvider from '../MCWSPersistenceProvider';
import mcws from '../../services/mcws/mcws';
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use the defined path 'services/mcws/mcws'

};
import MCWSPersistenceProvider from '../MCWSPersistenceProvider';
import MCWSUserContainerProvider from '../MCWSUserContainerProvider';
import mcws from '../../services/mcws/mcws';
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use the defined path 'services/mcws/mcws'

})
});
import MCWSPersistenceProvider from '../MCWSPersistenceProvider';
import mcws from '../../services/mcws/mcws';
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can use the defined path 'services/mcws/mcws'

});

// DO WE DELETE THIS TEST? I don't think we use this functionality anymore.
xit('provides a listing of namespaces when provided a namespace definition', async (done) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we do call getNamespacesFromMCWS so maybe this is still an okay unit test

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll take a look.

@jvigliotta
Copy link
Collaborator Author

Thanks for doing this. I have a couple comments inline.

Did all the other tests pass? I thought for sure the data product cell tests broke as a result of vue upgrade. Did we fix those elsewhere?

I'll look... I thought that was one of the first one's I did.

@jvigliotta jvigliotta requested a review from davetsay January 14, 2025 22:05
Copy link
Collaborator

@davetsay davetsay left a comment

Choose a reason for hiding this comment

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

Please link to an issue. Also, create issues to back fill the tests that were x-ed out, the window mocking one and the vue components ones. We should still retain the same unit test coverage and deprecate only when we get e2e test coverage.

@jvigliotta jvigliotta marked this pull request as ready for review January 14, 2025 23:09
@jvigliotta jvigliotta merged commit d0a03b8 into main Jan 14, 2025
4 checks passed
@jvigliotta jvigliotta deleted the update-tests branch January 14, 2025 23:21
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.

Update Unit Tests broken from updates
2 participants