Variables phase II, part I (#954)

* re-enable type checker 4 realz
* Update celery script `execute` node to accept nested variable declarations
* Disable test button on parameterized sequences (for now)
* Fix unnoticed NPE
* Add `deep-cover` for greater coverage accuracy
* Spec for bad variable assignment
pull/956/head
Rick Carlino 2018-08-09 18:31:22 -05:00 committed by GitHub
parent 46891a742d
commit 2caa1684d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 181 additions and 263 deletions

View File

@ -35,7 +35,9 @@ gem "zero_downtime_migrations"
group :development, :test do
gem "thin"
gem "capybara"
gem "deep-cover", "~> 0.4", require: false
gem "codecov", require: false
gem "simplecov"
gem "database_cleaner"
gem "factory_bot_rails"
gem "faker"
@ -48,7 +50,6 @@ group :development, :test do
gem "rspec-rails"
gem "rspec"
gem "selenium-webdriver"
gem "simplecov"
gem "smarf_doc", git: "https://github.com/RickCarlino/smarf_doc.git"
end

View File

@ -57,7 +57,11 @@ GEM
public_suffix (>= 2.0.2, < 4.0)
amq-protocol (2.3.0)
arel (9.0.0)
ast (2.4.0)
backports (3.11.3)
bcrypt (3.1.12)
binding_of_caller (0.8.0)
debug_inspector (>= 0.0.1)
builder (3.2.3)
bunny (2.11.0)
amq-protocol (~> 2.3.0)
@ -83,8 +87,21 @@ GEM
crass (1.0.4)
daemons (1.2.6)
database_cleaner (1.7.0)
debug_inspector (0.0.3)
declarative (0.0.10)
declarative-option (0.1.0)
deep-cover (0.6.2)
backports (>= 3.11.0)
binding_of_caller
bundler
highline
parser (~> 2.5.0)
pry
sass
slop (~> 4.0)
term-ansicolor
terminal-table
with_progress
delayed_job (4.1.5)
activesupport (>= 3.0, < 5.3)
delayed_job_active_record (4.1.3)
@ -158,6 +175,7 @@ GEM
signet (~> 0.7)
hashdiff (0.3.7)
hashie (3.5.7)
highline (2.0.0)
httpclient (2.8.3)
i18n (1.0.1)
concurrent-ruby (~> 1.0)
@ -207,6 +225,8 @@ GEM
mime-types
mimemagic (~> 0.3.0)
terrapin (~> 0.6.0)
parser (2.5.1.2)
ast (~> 2.4.0)
passenger (5.3.3)
rack
rake (>= 0.8.1)
@ -266,6 +286,9 @@ GEM
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rake (12.3.1)
rb-fsevent (0.10.3)
rb-inotify (0.9.10)
ffi (>= 0.5.0, < 2)
representable (3.0.4)
declarative (< 0.1.0)
declarative-option (< 0.2.0)
@ -300,7 +323,13 @@ GEM
rspec-support (~> 3.7.0)
rspec-support (3.7.1)
ruby-graphviz (1.2.3)
ruby-progressbar (1.10.0)
rubyzip (1.2.1)
sass (3.5.7)
sass-listen (~> 4.0.0)
sass-listen (4.0.0)
rb-fsevent (~> 0.9, >= 0.9.4)
rb-inotify (~> 0.9, >= 0.9.7)
scenic (1.4.1)
activerecord (>= 4.0.0)
railties (>= 4.0.0)
@ -322,6 +351,7 @@ GEM
skylight-core (= 2.0.2)
skylight-core (2.0.2)
activesupport (>= 4.2.0)
slop (4.6.2)
sprockets (3.7.2)
concurrent-ruby (~> 1.0)
rack (> 1, < 3)
@ -329,6 +359,10 @@ GEM
actionpack (>= 4.0)
activesupport (>= 4.0)
sprockets (>= 3.0.0)
term-ansicolor (1.6.0)
tins (~> 1.0)
terminal-table (1.8.0)
unicode-display_width (~> 1.1, >= 1.1.1)
terrapin (0.6.0)
climate_control (>= 0.0.3, < 1.0)
thin (1.7.2)
@ -337,9 +371,11 @@ GEM
rack (>= 1, < 3)
thor (0.19.4)
thread_safe (0.3.6)
tins (1.16.3)
tzinfo (1.2.5)
thread_safe (~> 0.1)
uber (0.1.0)
unicode-display_width (1.4.0)
url (0.3.2)
valid_url (0.0.4)
addressable
@ -351,6 +387,8 @@ GEM
websocket-driver (0.7.0)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.3)
with_progress (1.0.1)
ruby-progressbar (~> 1.4)
xpath (3.1.0)
nokogiri (~> 1.8)
zero_downtime_migrations (0.0.7)
@ -365,6 +403,7 @@ DEPENDENCIES
capybara
codecov
database_cleaner
deep-cover (~> 0.4)
delayed_job
delayed_job_active_record
devise

View File

@ -164,6 +164,7 @@ private
# Attempt 1:
# The device is using an HTTP client that does not provide a user-agent.
# We will assume this is an old FBOS version and set it to 0.0.0
# TODO UNTESTED CODE: simplecov counts this as tested. It is not.
return CalculateUpgrade::NULL if ua == NO_UA_FOUND
# Attempt 2:

View File

@ -103,6 +103,15 @@ module CeleryScript
run_additional_validations(value, key)
end
def type_check_parameter(var, expected)
data_type = var.args[:data_type].value
bad_var!(value, label, expected, actual) if !expected.include?(data_type)
end
def bad_var!(value, label, expected, actual)
value.invalidate!(T_MISMATCH % [label, expected, actual])
end
def validate_node_pairing(key, value)
actual = value.kind
allowed = corpus.fetchArg(key).allowed_values.map(&:to_s)
@ -114,15 +123,18 @@ module CeleryScript
# in depth type checking. We're not there yet, though.
# Currently we just need `resolve_variable!` to
# catch unbound identifiers
# data_type =
resolve_variable!(value)#.args[:data_type].value
# if !allowed_types.include?(data_type)
# # Did it reolve?
# # YES: Make sure it resolves to a `kind` from the list above.
# value.invalidate!(T_MISMATCH % [value.args["label"].value,
# allowed_types,
# data_type])
# end
var = resolve_variable!(value)
case var.kind
when "parameter_declaration"
type_check_parameter(var, allowed_types)
when "variable_declaration"
actual = var.args[:data_value].kind
unless allowed_types.include?(actual)
bad_var!(value, var.args[:label].value, allowed_types, actual)
end
else
raise ("Bad kind: " + var.kind)
end
end
ok = allowed.include?(actual)
raise TypeCheckError, (BAD_LEAF % [ value.kind,

View File

@ -17,8 +17,7 @@ class ServiceRunner
@channel.subscribe(block: true) do |info, _, payl|
@worker.process(info, payl.force_encoding("UTF-8"))
end
rescue OFFLINE_ERROR => e
rescue StandardError => e
rescue OFFLINE_ERROR, StandardError => e
unless e.is_a?(OFFLINE_ERROR)
Rollbar.error(e)
print CRASH_MSG

View File

@ -208,7 +208,7 @@ module CeleryScriptSettingsBag
.node(:channel, [:channel_name])
.node(:wait, [:milliseconds])
.node(:send_message, [:message, :message_type], [:channel])
.node(:execute, [:sequence_id])
.node(:execute, [:sequence_id], [:variable_declaration])
.node(:_if, [:lhs, :op, :rhs, :_then, :_else], [:pair])
.node(:sequence, [:version, :locals], STEPS)
.node(:home, [:speed, :axis], [])

View File

@ -25,8 +25,7 @@ class TokenIssuance < ApplicationRecord
id = "device_#{device_id}"
Transport::Mgmt.try(:close_connections_for_username, id)
end
rescue Faraday::ConnectionFailed
rescue Timeout::Error
rescue Faraday::ConnectionFailed, Timeout::Error
Rollbar.error("Failed to evict clients on token revocation")
end

View File

@ -1,257 +1,14 @@
# Use this hook to configure devise mailer, warden hooks and so forth.
# Many of these configuration options can be set straight in your model.
Devise.setup do |config|
# The secret key used by Devise. Devise uses this key to generate
# random tokens. Changing this key will render invalid all existing
# confirmation, reset password and unlock tokens in the database.
config.secret_key = ENV['DEVISE_SECRET']
# ==> Mailer Configuration
# Configure the e-mail address which will be shown in Devise::Mailer,
# note that it will be overwritten if you use your own mailer class
# with default "from" parameter.
config.mailer_sender = 'do-not-reply@farmbot.io'
# Configure the class responsible to send e-mails.
# config.mailer = 'Devise::Mailer'
# ==> ORM configuration
require 'devise/orm/active_record'
# ==> Configuration for any authentication mechanism
# Configure which keys are used when authenticating a user. The default is
# just :email. You can configure it to use [:username, :subdomain], so for
# authenticating a user, both parameters are required. Remember that those
# parameters are used only when authenticating and not when retrieving from
# session. If you need permissions, you should implement that in a before filter.
# You can also supply a hash where the value is a boolean determining whether
# or not authentication should be aborted when the value is not present.
# config.authentication_keys = [ :email ]
# Configure parameters from the request object used for authentication. Each entry
# given should be a request method and it will automatically be passed to the
# find_for_authentication method and considered in your model lookup. For instance,
# if you set :request_keys to [:subdomain], :subdomain will be used on authentication.
# The same considerations mentioned for authentication_keys also apply to request_keys.
# config.request_keys = []
# Configure which authentication keys should be case-insensitive.
# These keys will be downcased upon creating or modifying a user and when used
# to authenticate or find a user. Default is :email.
config.case_insensitive_keys = [ :email ]
# Configure which authentication keys should have whitespace stripped.
# These keys will have whitespace before and after removed upon creating or
# modifying a user and when used to authenticate or find a user. Default is :email.
config.strip_whitespace_keys = [ :email ]
# Tell if authentication through request.params is enabled. True by default.
# It can be set to an array that will enable params authentication only for the
# given strategies, for example, `config.params_authenticatable = [:database]` will
# enable it only for database (email + password) authentication.
# config.params_authenticatable = true
# Tell if authentication through HTTP Auth is enabled. False by default.
# It can be set to an array that will enable http authentication only for the
# given strategies, for example, `config.http_authenticatable = [:database]` will
# enable it only for database authentication. The supported strategies are:
# :database = Support basic authentication with authentication key + password
# config.http_authenticatable = false
# If http headers should be returned for AJAX requests. True by default.
# config.http_authenticatable_on_xhr = true
# The realm used in Http Basic Authentication. 'Application' by default.
# config.http_authentication_realm = 'Application'
# It will change confirmation, password recovery and other workflows
# to behave the same regardless if the e-mail provided was right or wrong.
# Does not affect registerable.
# config.paranoid = true
# By default Devise will store the user in session. You can skip storage for
# particular strategies by setting this option.
# Notice that if you are skipping storage for all authentication paths, you
# may want to disable generating routes to Devise's sessions controller by
# passing skip: :sessions to `devise_for` in your config/routes.rb
config.skip_session_storage = [:http_auth]
# By default, Devise cleans up the CSRF token on authentication to
# avoid CSRF token fixation attacks. This means that, when using AJAX
# requests for sign in and sign up, you need to get a new CSRF token
# from the server. You can disable this option at your own risk.
# config.clean_up_csrf_token_on_authentication = true
# ==> Configuration for :database_authenticatable
# For bcrypt, this is the cost for hashing the password and defaults to 10. If
# using other encryptors, it sets how many times you want the password re-encrypted.
#
# Limiting the stretches to just one in testing will increase the performance of
# your test suite dramatically. However, it is STRONGLY RECOMMENDED to not use
# a value less than 10 in other environments. Note that, for bcrypt (the default
# encryptor), the cost increases exponentially with the number of stretches (e.g.
# a value of 20 is already extremely slow: approx. 60 seconds for 1 calculation).
config.stretches = Rails.env.test? ? 1 : 10
# Setup a pepper to generate the encrypted password.
# config.pepper = 'ad102f8786d03074ff9af32989cac7ff9f6ec9a08fb5a5177805c537ed2f03f7910e8e473514bef15c6641666507d7f27413b868d93661d55cfa428026911ece'
# ==> Configuration for :confirmable
# A period that the user is allowed to access the website even without
# confirming their account. For instance, if set to 2.days, the user will be
# able to access the website for two days without confirming their account,
# access will be blocked just in the third day. Default is 0.days, meaning
# the user cannot access the website without confirming their account.
# config.allow_unconfirmed_access_for = 2.days
# A period that the user is allowed to confirm their account before their
# token becomes invalid. For example, if set to 3.days, the user can confirm
# their account within 3 days after the mail was sent, but on the fourth day
# their account can't be confirmed with the token any more.
# Default is nil, meaning there is no restriction on how long a user can take
# before confirming their account.
# config.confirm_within = 3.days
# If true, requires any email changes to be confirmed (exactly the same way as
# initial account confirmation) to be applied. Requires additional unconfirmed_email
# db field (see migrations). Until confirmed, new email is stored in
# unconfirmed_email column, and copied to email column on successful confirmation.
config.reconfirmable = true
# Defines which key will be used when confirming an account
# config.confirmation_keys = [ :email ]
# ==> Configuration for :rememberable
# The time the user will be remembered without asking for credentials again.
# config.remember_for = 2.weeks
# Invalidates all the remember me tokens when the user signs out.
config.expire_all_remember_me_on_sign_out = true
# If true, extends the user's remember period when remembered via cookie.
# config.extend_remember_period = false
# Options to be passed to the created cookie. For instance, you can set
# secure: true in order to force SSL only cookies.
# config.rememberable_options = {}
# ==> Configuration for :validatable
# Range for password length.
config.password_length = 8..128
# Email regex used to validate email formats. It simply asserts that
# one (and only one) @ exists in the given string. This is mainly
# to give user feedback and not to assert the e-mail validity.
# config.email_regexp = /\A[^@]+@[^@]+\z/
# ==> Configuration for :timeoutable
# The time you want to timeout the user session without activity. After this
# time the user will be asked for credentials again. Default is 30 minutes.
# config.timeout_in = 30.minutes
# If true, expires auth token on session timeout.
# config.expire_auth_token_on_timeout = false
# ==> Configuration for :lockable
# Defines which strategy will be used to lock an account.
# :failed_attempts = Locks an account after a number of failed attempts to sign in.
# :none = No lock strategy. You should handle locking by yourself.
# config.lock_strategy = :failed_attempts
# Defines which key will be used when locking and unlocking an account
# config.unlock_keys = [ :email ]
# Defines which strategy will be used to unlock an account.
# :email = Sends an unlock link to the user email
# :time = Re-enables login after a certain amount of time (see :unlock_in below)
# :both = Enables both strategies
# :none = No unlock strategy. You should handle unlocking by yourself.
# config.unlock_strategy = :both
# Number of authentication tries before locking an account if lock_strategy
# is failed attempts.
# config.maximum_attempts = 20
# Time interval to unlock the account if :time is enabled as unlock_strategy.
# config.unlock_in = 1.hour
# Warn on the last attempt before the account is locked.
# config.last_attempt_warning = false
# ==> Configuration for :recoverable
#
# Defines which key will be used when recovering the password for an account
# config.reset_password_keys = [ :email ]
# Time interval you can reset your password with a reset password key.
# Don't put a too small interval or your users won't have the time to
# change their passwords.
config.reset_password_within = 6.hours
# ==> Configuration for :encryptable
# Allow you to use another encryption algorithm besides bcrypt (default). You can use
# :sha1, :sha512 or encryptors from others authentication tools as :clearance_sha1,
# :authlogic_sha512 (then you should set stretches above to 20 for default behavior)
# and :restful_authentication_sha1 (then you should set stretches to 10, and copy
# REST_AUTH_SITE_KEY to pepper).
#
# Require the `devise-encryptable` gem when using anything other than bcrypt
# config.encryptor = :sha512
# ==> Scopes configuration
# Turn scoped views on. Before rendering "sessions/new", it will first check for
# "users/sessions/new". It's turned off by default because it's slower if you
# are using only default views.
# config.scoped_views = false
# Configure the default scope given to Warden. By default it's the first
# devise role declared in your routes (usually :user).
# config.default_scope = :user
# Set this configuration to false if you want /users/sign_out to sign out
# only the current scope. By default, Devise signs out all scopes.
# config.sign_out_all_scopes = true
# ==> Navigation configuration
# Lists the formats that should be treated as navigational. Formats like
# :html, should redirect to the sign in page when the user does not have
# access, but formats like :xml or :json, should return 401.
#
# If you have any extra navigational formats, like :iphone or :mobile, you
# should add them to the navigational formats lists.
#
# The "*/*" below is required to match Internet Explorer requests.
# config.navigational_formats = ['*/*', :html]
# The default HTTP method used to sign out a resource. Default is :delete.
config.sign_out_via = [:delete, :get]
# ==> OmniAuth
# Add a new OmniAuth provider. Check the wiki for more information on setting
# up on your models and hooks.
# config.omniauth :github, 'APP_ID', 'APP_SECRET', scope: 'user,public_repo'
# ==> Warden configuration
# If you want to use other strategies, that are not supported by Devise, or
# change the failure app, you can configure them inside the config.warden block.
#
# config.warden do |manager|
# manager.intercept_401 = false
# manager.default_strategies(scope: :user).unshift :some_external_strategy
# end
# ==> Mountable engine configurations
# When using Devise inside an engine, let's call it `MyEngine`, and this engine
# is mountable, there are some extra configurations to be taken into account.
# The following options are available, assuming the engine is mounted as:
#
# mount MyEngine, at: '/my_engine'
#
# The router that invoked `devise_for`, in the example above, would be:
# config.router_name = :my_engine
#
# When using omniauth, Devise cannot automatically set Omniauth path,
# so you need to do it manually. For the users scope, it would be:
# config.omniauth_path_prefix = '/my_engine/users/auth'
end

View File

@ -52,7 +52,7 @@
"css-loader": "1.0.0",
"enzyme": "^3.1.0",
"enzyme-adapter-react-16": "^1.1.0",
"farmbot": "6.4.3",
"farmbot": "6.5.0-rc1",
"farmbot-toastr": "^1.0.3",
"fastclick": "^1.0.6",
"file-loader": "1.1.11",

View File

@ -217,4 +217,72 @@ describe CeleryScript::Checker do
expect(chk.valid?).to be false
expect(chk.error.message).to include("not a valid package")
end
it "handles good variable declarations" do
ast = {
kind: "sequence",
args: {
version: 20180209,
locals: {
kind: "scope_declaration",
:args=>{},
body: [
{
kind: "variable_declaration",
args: {
label: "parent",
data_value: { kind: "coordinate", args: { x: 0, y: 0, z: 0 } }
}
}
]
}
},
body: [
{
kind: "move_absolute",
args: {
speed: 100,
location: { kind: "identifier", args: { label: "parent" } },
offset: { kind: "coordinate", args: { x: 0, y: 0, z: 0} }
}
}
]
}
tree = CeleryScript::AstNode.new(ast)
chk = CeleryScript::Checker.new(tree, corpus)
expect(chk.valid?).to be true
end
it "handles bad variable declarations" do
ast = {
kind: "sequence",
args: {
version: 20180209,
locals: {
kind: "scope_declaration",
:args=>{},
body: [
{
kind: "variable_declaration",
args: { label: "parent", data_value: { kind: "nothing", args: { } } }
}
]
}
},
body: [
{
kind: "move_absolute",
args: {
speed: 100,
location: { kind: "identifier", args: { label: "parent" } },
offset: { kind: "coordinate", args: { x: 0, y: 0, z: 0} }
}
}
]
}
tree = CeleryScript::AstNode.new(ast)
chk = CeleryScript::Checker.new(tree, corpus)
expect(chk.valid?).to be false
expect(chk.error.message).to include('but got "nothing"')
end
end

View File

@ -1,6 +1,7 @@
DO_INTEGRATION = !!ENV["RUN_CAPYBARA"]
ENV["MQTT_HOST"] = "blooper.io"
ENV["OS_UPDATE_SERVER"] = "http://non_legacy_update_url.com"
require "deep_cover/builtin_takeover"
require "simplecov"
#Ignore anything with the word "spec" in it. No need to test your tests.
SimpleCov.start do

View File

@ -0,0 +1,33 @@
import { fakeSequence } from "../../__test_support__/fake_state/resources";
import { TaggedSequence } from "farmbot";
import { isParameterized } from "../is_parameterized";
type Sequence = TaggedSequence["body"];
type Locals = Sequence["args"]["locals"]["body"];
describe("isParameterized()", () => {
function sequence(decl: Locals): Sequence {
const { body } = fakeSequence();
body.args.locals.body = decl;
return body;
}
it("returns true when there are parameters", () => {
const hasParent = sequence([
{
kind: "parameter_declaration",
args: {
label: "parent",
data_type: "Point"
}
}
]);
expect(isParameterized(hasParent)).toBeTruthy();
});
it("returns false when there are no parameters", () => {
expect(isParameterized(sequence(undefined))).toBeFalsy();
expect(isParameterized(sequence([]))).toBeFalsy();
});
});

View File

@ -0,0 +1,7 @@
import { TaggedSequence } from "farmbot";
/** Determine if a sequence has parameters */
export function isParameterized(s: TaggedSequence["body"]) {
const { body } = s.args.locals;
return !!(body && body.length);
}

View File

@ -2,6 +2,7 @@ import * as React from "react";
import { t } from "i18next";
import { SyncStatus } from "farmbot/dist";
import { TaggedSequence } from "farmbot";
import { isParameterized } from "./is_parameterized";
export interface TestBtnProps {
/** Callback fired ONLY if synced. */
@ -15,7 +16,7 @@ export interface TestBtnProps {
export function TestButton({ onClick, onFail, syncStatus, sequence }: TestBtnProps) {
const isSynced = syncStatus === "synced";
const isSaved = !sequence.specialStatus;
const canTest = isSynced && isSaved;
const canTest = isSynced && isSaved && !isParameterized(sequence.body);
const className = canTest ? "orange" : "pseudo-disabled";
const clickHandler = () => (canTest) ?

View File

@ -2417,9 +2417,9 @@ farmbot-toastr@^1.0.0, farmbot-toastr@^1.0.3:
farmbot-toastr "^1.0.0"
typescript "^2.3.4"
farmbot@6.4.3:
version "6.4.3"
resolved "https://registry.yarnpkg.com/farmbot/-/farmbot-6.4.3.tgz#08f6c361e006410aac87dbba28d3c1a291a458b2"
farmbot@6.5.0-rc1:
version "6.5.0-rc1"
resolved "https://registry.yarnpkg.com/farmbot/-/farmbot-6.5.0-rc1.tgz#ad8782b278d743c6ae4051845385de05f825c229"
dependencies:
mqtt "2.15.0"